klsince commented on code in PR #13285: URL: https://github.com/apache/pinot/pull/13285#discussion_r1635397699
########## 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: good question, and this method was refactor earlier on to firstly take snapshots for segments already having snapshot on disk, and then take snapshot for those w/o snapshots (i.e. why there are two for-loops in the method), to make sure that even if failure happens in the middle of taking snapshots, the snapshots ever put on disk continue to contain a set of disjoint valid docs. -- 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