deepthi912 commented on code in PR #19028:
URL: https://github.com/apache/pinot/pull/19028#discussion_r3626759478


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -1006,26 +1046,38 @@ protected void doTakeSnapshot() {
       }
     }
     // If we have skipped any segments in the previous for-loop, we should 
skip the next for-loop, basically to not
-    // add new snapshot files on disk. This ensures all the validDocIds & 
queryable docIds snapshots kept on disk are
-    // still disjoint with each other, although some of them may have become 
stale, i.e. tracking more valid docs than
-    // expected.
+    // add new snapshot files on disk. This ensures all the validDocIds 
snapshots kept on disk are still disjoint
+    // with each other, although some of them may have become stale, i.e. 
tracking more valid docs than expected.
     if (!isSegmentSkipped) {

Review Comment:
   Good change, but I am thinking if there is a possibility of us running into 
a condition of not able to acquire the lock. We can test out this change and 
see how it behaves in the clusters



-- 
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]

Reply via email to