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


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -905,18 +905,25 @@ protected void doTakeSnapshot() {
           segmentsWithoutSnapshot.add(immutableSegment);
           continue;
         }
+        ThreadSafeMutableRoaringBitmap queryableDocIdsBitMap = null;
+        ThreadSafeMutableRoaringBitmap validDocIdsBitMap =
+            immutableSegment.getValidDocIds() == null ? new 
ThreadSafeMutableRoaringBitmap()

Review Comment:
   Sometimes when loading a snapshot file: `loadDocIdsFromSnapshot` In many 
cases, we are handling it creating it as new ThreadSafeMutableRoaringBitmap() 
for null values, for the safer side I added this check to handle null values. 
   Also let's say we have segment compaction tasks running on a segment 
removing all the validDocs, but until next iteration this segment will have 0 
validDocs marking it EmptySegment. In such cases we might end up having null 
values I think  where empty segments remain in Pinot until they are deleted by 
the next cycle of task generation.  
   



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