xiangfu0 commented on code in PR #20010:
URL: https://github.com/apache/pinot/pull/20010#discussion_r4563736970
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java:
##########
@@ -121,10 +121,22 @@ protected void processTable(String tableNameWithType) {
return;
}
+ processTable(tableConfig, new TableRunContext(tableNameWithType));
+ }
+
+ /// Runs the per-table work of one run. `context` memoizes the segment ZK
metadata list and the deep-store directory
+ /// listing used by time retention and the untracked-segment sweep. A
subclass that adds per-table work overrides
+ /// this method and reads through the same context before calling `super`:
it then shares the reads those steps
+ /// make, and reads on its own only where they skip one (a non-APPEND
OFFLINE table, a table without retention,
+ /// untracked-segment deletion off). Hybrid-strategy and size-based
retention read ZK themselves. The context is
+ /// released after time retention, which can delete segments, so a read
after `super` sees fresh state.
+ protected void processTable(TableConfig tableConfig, TableRunContext
context) {
// Manage normal table retention except segment lineage cleanup.
// The reason of separating the logic is that REFRESH only table will be
skipped in the first part,
// whereas the segment lineage cleanup needs to be handled.
- manageRetentionForTable(tableConfig);
+ manageRetentionForTable(tableConfig, context);
+ // The memoized reads predate the deletes above, and the steps below read
what they need themselves.
+ context.release();
Review Comment:
Nit (optional): if `manageRetentionForTable` throws after a partial delete,
`release()` never runs. A subclass that catches the exception from `super`
would then still read the pre-delete snapshot, which contradicts the "read
after `super` sees fresh state" doc. A `try`/`finally` around the call would
make that guarantee unconditional.
--
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]
