deemoliu commented on code in PR #10047:
URL: https://github.com/apache/pinot/pull/10047#discussion_r1099384821


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -217,6 +226,17 @@ public void replaceSegment(ImmutableSegment segment, 
@Nullable ThreadSafeMutable
         }
         addOrReplaceSegment((ImmutableSegmentImpl) segment, validDocIds, 
recordInfoIterator, oldSegment,
             validDocIdsForOldSegment);
+
+        // remove expired primary keys from primary key indexes.
+        if (_upsertTTLStrategy != null) {
+          removeExpiredPrimaryKeys(segment, recordInfoIterator, 
_upsertTTLStrategy.getTtlInMs());
+          MutableRoaringBitmap validDocIdsSnapshot =

Review Comment:
   > Do we plan to implement the "Remove synchronously" approach described in 
the design doc? If so, it cannot be done in the replace segment because it is 
too late, and the new consuming segment might already be created. We will need 
to add a separate method which is invoked after the segment is getting 
committed, and before the new segment starts consuming.
   
   @Jackie-Jiang is there some examples that invoked after the segment is 
getting committed, and before the new segment starts consuming? 



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

Reply via email to