FrankChen021 commented on code in PR #19672:
URL: https://github.com/apache/druid/pull/19672#discussion_r3558725168
##########
server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java:
##########
@@ -404,6 +404,15 @@ tableName, getPayloadType(), getQuoteString(),
getCollation()
"start"
)
);
+ // Covering index for the used-segment ID scan performed on every metadata
+ // cache sync (SELECT id, dataSource, used_status_last_updated WHERE
used=true).
+ // id rides along as the implicit primary key, so this makes the scan
+ // index-only and avoids reading the payload-bearing clustered index.
+ createIndex(
+ tableName,
+ "IDX_%S_USED_USLU_DATASOURCE",
+ List.of("used", "used_status_last_updated", "dataSource")
Review Comment:
[P2] Include id in the portable covering index
The assumption that a secondary index implicitly carries the primary key is
specific to engines such as InnoDB. PostgreSQL stores the primary key in a
separate index, so this index still requires a heap lookup for every selected
`id`, defeating the intended index-only scan on a supported metadata backend.
Include `id` explicitly or provide backend-specific index definitions.
##########
server/src/main/java/org/apache/druid/metadata/segment/cache/HeapMemorySegmentMetadataCache.java:
##########
@@ -627,19 +656,35 @@ private void markCacheSynced(DateTime syncStartTime)
}
}
);
+ removedDatasources.add(dataSource);
} else {
emitMetric(dataSource, Metric.CACHED_INTERVALS,
stats.getNumIntervals());
emitMetric(dataSource, Metric.CACHED_USED_SEGMENTS,
stats.getNumUsedSegments());
emitMetric(dataSource, Metric.CACHED_UNUSED_SEGMENTS,
stats.getNumUnusedSegments());
emitMetric(dataSource, Metric.CACHED_PENDING_SEGMENTS,
stats.getNumPendingSegments());
- datasourceToUsedSegments.put(dataSource,
cache.findUsedSegmentsOverlappingAnyOf(List.of()));
+ // Only materialize the (potentially large) used-segment set for
datasources
+ // that must be rebuilt into the snapshot.
+ if (!incremental || dirtyDatasources.contains(dataSource)) {
Review Comment:
[P1] Track write-through cache mutations as dirty
Incremental rebuilding only materializes datasources returned by
`updateSegmentIdsInCache`. However, `CachedSegmentMetadataTransaction` applies
successful inserts and deletes directly to the datasource cache. On the next
poll, the database and cache already match, so `syncSegmentIds` reports no
expired or deleted IDs and the datasource is skipped here. Consequently, newly
inserted segments can remain absent from the published snapshot, and partial
deletions can remain present, indefinitely until an unrelated update dirties
that datasource. Record used-segment mutations from `writeCacheForDataSource`
or otherwise compare against the prior snapshot before reusing it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
