deemoliu commented on code in PR #10047:
URL: https://github.com/apache/pinot/pull/10047#discussion_r1191588988
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentImpl.java:
##########
@@ -159,7 +159,11 @@ public void deleteValidDocIdsSnapshot() {
}
}
- private File getValidDocIdsSnapshotFile() {
+ private File getValidDocIdsSnapshotFile(boolean isTTLEnabled) {
+ if (isTTLEnabled) {
+ return new
File(SegmentDirectoryPaths.findSegmentDirectory(_segmentMetadata.getIndexDir()),
Review Comment:
@KKcorps thanks for the code review and pointing out this part.
I think I missed this part in the code, `loadValidDocIdsFromSnapshot(true)`
and `deleteValidDocIdsSnapshot(true)` should be called here when TTLConfig
exists. I will add it.
I also think about this part again, I feel snapshot feature and TTLConfig
and enableSnapshot might not need to be enabled together, otherwise we will
store duplicated snapshots, and the snapshot persisted by the snapshotEnabled
will never be used by TTL feature. If we go through this route, we will handle
three scenarios
1) TTL config enabled
2) snapshot enabled
3) both are not enabled.
I will also add validation to avoid both config enabled in upsertConfig.
--
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]
