jsedding commented on code in PR #3082:
URL: https://github.com/apache/jackrabbit-oak/pull/3082#discussion_r3795165311
##########
oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CheckpointCompactorEfficiencyTest.java:
##########
@@ -77,6 +78,12 @@
*/
class CheckpointCompactorEfficiencyTest {
+ private static final int WIDTH = 1000;
+
+ // Nodes a correct retry may compact beyond the changed children (their
ancestor spine plus the
+ // concurrent-checkpoint structure).
Review Comment:
I don't understand this comment.
Also, what does "spine" mean in this context?
##########
oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CheckpointCompactor.java:
##########
Review Comment:
Your code is perfectly correct. I was wondering, however, how the intention
of the code could be expressed more clearly. Below is my attempt at this.
In words: to compact each checkpoint (and in the end head-state), we iterate
over the `superRoots`. The root states of the checkpoints are replayed
sequentially, in order to maximise the reused records in each compacted
`ontoRoot`.
So for each iteration the previous `afterRoot` becomes the new `beforeRoot`,
and the previous `compacted` state becomes the new `ontoRoot`.
For the `afterRoot`, the state for the current iteration is always retrieved
from the current checkpoint being processed.
WDYT? Clearly, whether or not the code is intuitive is subjective. So feel
free to proceed as you like.
```
NodeState baseRoot = getRoot(before);
NodeState ontoRoot = getRoot(onto);
for (String path : superRoots) {
NodeState afterSuperRoot = getDescendant(after, path,
NodeState::getChildNode);
NodeState afterRoot = getRoot(afterSuperRoot);
CompactedNodeState compacted = compactRootState(baseRoot,
afterRoot, ontoRoot, hardCanceller, softCanceller);
if (compacted == null) {
// only happens for hard cancellation
return null;
}
Validate.checkState(compacted.isComplete() ||
isCancelled(softCanceller),
"compaction must be complete unless cancelled");
NodeBuilder builder = getDescendant(rootBuilder, path,
NodeBuilder::child);
builder.setChildNode(ROOT, compacted);
if (path.startsWith(CHECKPOINTS + '/')) {
compactCheckpointMetadata(builder, afterSuperRoot);
}
if (isCancelled(softCanceller)) {
break;
}
baseRoot = afterRoot; // afterRoot is the baseRoot of the next
iteration
ontoRoot = compacted;
}
```
--
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]
