Jackie-Jiang commented on code in PR #13992: URL: https://github.com/apache/pinot/pull/13992#discussion_r1762165645
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java: ########## @@ -1200,12 +1219,45 @@ public void trackSegmentForUpsertView(IndexSegment segment) { if (_upsertViewManager != null) { _upsertViewManager.trackSegment(segment); } + if (segment instanceof MutableSegment) { + trackNewlyAddedSegment(segment); + } } @Override public void untrackSegmentForUpsertView(IndexSegment segment) { if (_upsertViewManager != null) { _upsertViewManager.untrackSegment(segment); } + if (segment instanceof MutableSegment) { + untrackNewlyAddedSegment(segment); + } + } + + @VisibleForTesting + void trackNewlyAddedSegment(IndexSegment segment) { + if (_newSegmentTrackingTimeMs > 0) { + _newlyAddedSegments.put(segment.getSegmentName(), -1L); + } + } + + @VisibleForTesting + void untrackNewlyAddedSegment(IndexSegment segment) { Review Comment: I understand why we want to track the segment before processing the segment addition, but why we want to untrack it after it is added? Seems like the intention is to always include the segment when it is being added, then start counting timeout after it is fully added? IMO that might not be necessary because we use this timeout to give some buffer time to broker to reflect the IS change. Can you directly track the segment with timeout before adding it? Or am I missing something here? -- 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