klsince commented on code in PR #13285: URL: https://github.com/apache/pinot/pull/13285#discussion_r1635387402
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java: ########## @@ -874,19 +879,32 @@ protected void doTakeSnapshot() { numConsumingSegments++; continue; } - ImmutableSegmentImpl immutableSegment = (ImmutableSegmentImpl) segment; - if (!immutableSegment.hasValidDocIdsSnapshotFile()) { - segmentsWithoutSnapshot.add(immutableSegment); + if (!_updatedSegmentsSinceLastSnapshot.contains(segment)) { + // if no updates since last snapshot then skip continue; } - immutableSegment.persistValidDocIdsSnapshot(); - numImmutableSegments++; - numPrimaryKeysInSnapshot += immutableSegment.getValidDocIds().getMutableRoaringBitmap().getCardinality(); + try { + ImmutableSegmentImpl immutableSegment = (ImmutableSegmentImpl) segment; + if (!immutableSegment.hasValidDocIdsSnapshotFile()) { + segmentsWithoutSnapshot.add(immutableSegment); + continue; + } + immutableSegment.persistValidDocIdsSnapshot(); + _updatedSegmentsSinceLastSnapshot.remove(segment); + numImmutableSegments++; + numPrimaryKeysInSnapshot += immutableSegment.getValidDocIds().getMutableRoaringBitmap().getCardinality(); + } catch (Exception e) { + _logger.warn("Caught exception while taking snapshot for segment: {}, skipping", segment.getSegmentName(), e); Review Comment: `preloadSegments()` method filter out the segments with bitmap snapshot and then add them with `addSegmentWithoutUpsert()`, so it's important to ensure that those segments' bitmap snapshots as persisted on disk should be disjoint (i.e. not contain duplicate PKs). And with this assumption, the order of loading those segments during this preloading phase doesn't matter. Once preloading finishes, the remaining segments are loaded normally, i.e. doing read-check-write flow. -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org