xiangfu0 commented on code in PR #19998:
URL: https://github.com/apache/pinot/pull/19998#discussion_r4368384467
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -1410,7 +1419,20 @@ protected void removeDocId(IndexSegment segment, int
docId) {
protected void trackUpdatedSegmentsSinceLastSnapshot(IndexSegment segment) {
if (_enableSnapshot && segment instanceof ImmutableSegmentImpl
immutableSegment) {
- _updatedSegmentsSinceLastSnapshot.add(immutableSegment);
+ markSegmentUpdated(immutableSegment);
+ }
+ }
+
+ private void markSegmentUpdated(ImmutableSegmentImpl segment) {
+ // Publish the marker before reading metadata so a concurrent snapshot
cannot classify this segment as unchanged.
+ if (_updatedSegmentsSinceLastSnapshot.putIfAbsent(segment,
TTL_WATERMARK_NOT_SET) != null
Review Comment:
Confirmed. The sentinel is intentional when the read lock cannot be
acquired: it prevents a stale snapshot from advancing the persisted TTL
watermark past an unreadable segment. A later mark in the same round does not
need to retry; the segment is retried in a later snapshot round, and a
successful snapshot clears its marker. I am keeping the conservative behavior.
--
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]
