deepthi912 commented on code in PR #19028:
URL: https://github.com/apache/pinot/pull/19028#discussion_r3619829498
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -1013,13 +1026,22 @@ protected void doTakeSnapshot() {
for (ImmutableSegmentImpl segment : segmentsWithoutSnapshot) {
String segmentName = segment.getSegmentName();
Lock segmentLock = tableDataManager.getSegmentLock(segmentName);
+ // Unlike the previous loop, skipping a segment here on lock
contention is benign: no snapshot file is
+ // written for it, so the snapshots kept on disk remain disjoint, and
the segment stays tracked as updated
+ // to be retried in the next snapshot round.
boolean locked = segmentLock.tryLock();
if (!locked) {
_logger.warn("Could not get segmentLock to take snapshot for
segment: {} w/o snapshot, skipping",
segmentName);
continue;
}
try {
+ // The segment can be replaced or removed by another thread after it
was collected in the previous loop.
+ // The replaced segment object no longer owns the segment directory,
so skip persisting its bitmaps.
+ if (!_trackedSegments.contains(segment)) {
Review Comment:
We anyway get this segment from _trackedSegments list right, why are we
checking for this condition again?
--
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]
