walterddr commented on code in PR #10928: URL: https://github.com/apache/pinot/pull/10928#discussion_r1235940432
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java: ########## @@ -61,8 +63,12 @@ public abstract class BasePartitionUpsertMetadataManager implements PartitionUps protected final ServerMetrics _serverMetrics; protected final Logger _logger; - @VisibleForTesting - public final Set<IndexSegment> _replacedSegments = ConcurrentHashMap.newKeySet(); + // Tracks all the segments managed by this manager (excluding EmptySegment) + protected final Set<IndexSegment> _trackedSegments = ConcurrentHashMap.newKeySet(); + + // NOTE: We do not persist snapshot on the first consuming segment because most segments might not be loaded yet + protected volatile boolean _gotFirstConsumingSegment = false; + protected final ReadWriteLock _snapshotLock; Review Comment: seems like all readLock are follow by start operation. do we still need the start operation? b/c there couldn't been any concurrent operations with the new lock right? -- 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