[GitHub] [pinot] walterddr commented on a diff in pull request #10928: Take upsert snapshot when creating new consuming segment

["walterddr (via GitHub)" <gi...@apache.org>]

walterddr commented on code in PR #10928:
URL: https://github.com/apache/pinot/pull/10928#discussion_r1235939623


##########
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:
   this lock is used (read-wise) by 3 methods `add/remove/replaceSegment`, therefore should this lock be private? 
   



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org