cryptoe commented on code in PR #17025:
URL: https://github.com/apache/druid/pull/17025#discussion_r1751285956
##########
server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java:
##########
@@ -431,10 +431,7 @@ public Map<SegmentId, AvailableSegmentMetadata>
getSegmentMetadataSnapshot()
@Nullable
public AvailableSegmentMetadata getAvailableSegmentMetadata(String
datasource, SegmentId segmentId)
{
- if (!segmentMetadataInfo.containsKey(datasource)) {
- return null;
- }
- return segmentMetadataInfo.get(datasource).get(segmentId);
+ return segmentMetadataInfo.getOrDefault(datasource, new
ConcurrentSkipListMap<>()).get(segmentId);
Review Comment:
Why would you create a new object if its not used. Less GC that way.
Isn't the older code more performant ?
##########
server/src/main/java/org/apache/druid/segment/metadata/AbstractSegmentMetadataCache.java:
##########
@@ -732,18 +718,35 @@ public Set<SegmentId> refreshSegmentsForDataSource(final
String dataSource, fina
log.debug("Refreshing metadata for datasource[%s].", dataSource);
+ final Set<SegmentId> retVal = new HashSet<>();
+
+ ConcurrentSkipListMap<SegmentId, AvailableSegmentMetadata>
datasourceSegments = segmentMetadataInfo.get(dataSource);
+ // this datasource no longer exists, skip refresh
+ if (datasourceSegments == null) {
+ return retVal;
+ }
+
+ // Skip refreshing tombstone segments. These segments lack data or column
information.
+ // Additionally, segment metadata queries, which are not yet implemented
for tombstone segments
+ // (see: https://github.com/apache/druid/pull/12137) do not provide
metadata for tombstones,
+ // leading to indefinite refresh attempts for these segments.
+ Set<SegmentId> segmentsWithoutTombstone =
Review Comment:
Why do we need to materialize this.
Why can't we return a iterable which just skips the tombstone segments ?
We can always increment counters there no ?
--
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]
