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


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -426,15 +434,15 @@ protected GenericRow doUpdateRecord(GenericRow record, 
RecordInfo recordInfo) {
           if (!recordInfo.isDeleteRecord()
               && 
recordInfo.getComparisonValue().compareTo(recordLocation.getComparisonValue()) 
>= 0) {
             IndexSegment currentSegment = recordLocation.getSegment();
-            ThreadSafeMutableRoaringBitmap currentQueryableDocIds = 
currentSegment.getQueryableDocIds();
             int currentDocId = recordLocation.getDocId();
-            if (currentQueryableDocIds == null || 
currentQueryableDocIds.contains(currentDocId)) {
+            // Read lock: currentSegment cannot be destroyed while LazyRow 
reads its columns. A consuming segment needs
+            // no lock: it is destroyed only after 
replaceSegment()/removeSegment() has moved or dropped every location
+            // pointing at it, and those run under the same per-key compute as 
this read.
+            if (tryAcquireSegmentReadLock(currentSegment)) {

Review Comment:
   Readers don't block readers. It's a shared lock — any number of updates can 
read the same segment at once.
   Each segment owns its lock, so two segments in the same partition are fully 
independent. Consumers don't wait for teardown. The write lock covers one 
boolean; closing indexes and unmapping memory happen after it's released. A 
consumer arriving mid-destroy takes the lock freely, sees the flag, and skips. 
So this change has to be light weight IMO.
   The lock costs 4 ns per update — at 10M records/s across 32 partitions 
that's 4% of a single CPU core, with no allocation and no lag.



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