New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improving retention manager to handle segment lineage clean-up #5828
Conversation
1. Added the logic to handle segment lineage clean-up in the retention manager. 2. Added the unit test
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Show resolved
Hide resolved
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
2a5b3d4
to
8bf18a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to check segmentsTo
for COMPLETED
. We can delete the lineage entry once all the segments in segmentsFrom
are already deleted (not in the current round).
segmentsToDelete.addAll(lineageEntry.getSegmentsFrom()); | ||
|
||
// The lineage entry with 'COMPLETED' state can only be safely removed when both segmentFrom & segmentTo | ||
// are all removed from the table. | ||
if (Collections.disjoint(segmentsForTable, lineageEntry.getSegmentsFrom()) && Collections | ||
.disjoint(segmentsForTable, lineageEntry.getSegmentsTo())) { | ||
segmentLineage.deleteLineageEntry(lineageEntryId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify the logic as following, same for IN_PROGRESS
segmentsToDelete.addAll(lineageEntry.getSegmentsFrom()); | |
// The lineage entry with 'COMPLETED' state can only be safely removed when both segmentFrom & segmentTo | |
// are all removed from the table. | |
if (Collections.disjoint(segmentsForTable, lineageEntry.getSegmentsFrom()) && Collections | |
.disjoint(segmentsForTable, lineageEntry.getSegmentsTo())) { | |
segmentLineage.deleteLineageEntry(lineageEntryId); | |
} | |
Set<String> sourceSegments = new HashSet<>(lineageEntry.getSegmentsFrom()); | |
sourceSegments.retainAll(segmentsForTable); | |
if (sourceSegments.isEmpty()) { | |
segmentLineage.deleteLineageEntry(lineageEntryId); | |
} else { | |
segmentsToDelete.addAll(sourceSegments); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed. Please double check the logic for IN_PROGRESS
as well.
...troller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
Outdated
Show resolved
Hide resolved
8bf18a7
to
7dc5b3b
Compare
7dc5b3b
to
3a637b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
retention manager.