klsince commented on code in PR #11020:
URL: https://github.com/apache/pinot/pull/11020#discussion_r1250232760
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -190,7 +192,11 @@ public void addSegment(ImmutableSegmentImpl segment,
@Nullable ThreadSafeMutable
if (queryableDocIds == null && _deleteRecordColumn != null) {
queryableDocIds = new ThreadSafeMutableRoaringBitmap();
}
- addOrReplaceSegment(segment, validDocIds, queryableDocIds,
recordInfoIterator, null, null);
+ if (_tableUpsertMetadataManager.isPreloadingSegment(segmentName)) {
Review Comment:
hmm.. after some thoughts, I could not figure out how to make the changes
you proposed.
Basically, if we call `preloadSegment(ImmutableSegment segment)` as part of
handleUpsert(), which is part of addSegment(ImmutableSegment segment))` of
`RealtimeTableDataManager`. I think that's too late for the snapshot existence
check, because the segment is already fully loaded as the `ImmutableSegment`
object is already created. The logic in current PR can skip the segment if it
doesn't have the snapshot file.
--
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]
