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