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: [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]
