Let TaskDeduplicator callers decide which executions to join - #30516
Open
fmeum wants to merge 4 commits into
Open
Let TaskDeduplicator callers decide which executions to join#30516fmeum wants to merge 4 commits into
fmeum wants to merge 4 commits into
Conversation
TaskDeduplicator deduplicated executions by key alone, so callers with different requirements either shared an execution that didn't satisfy all of them or had to be separated into different keys, which also gives up sharing in the direction where it is safe. Every execution is now started with caller-provided attributes and every caller supplies a predicate over them, so the join relation can be asymmetric: a caller that needs little joins an execution that produces more, while a caller that needs more starts its own and takes over as the execution that subsequent callers see. The decision is made inside a single ConcurrentHashMap.compute, which also lets the retry loop around a concurrently canceled execution go away. executeIfNew and executeUnconditionally are replaced by a single execute. MerkleTreeComputer's BlobPolicy is such a relation: a KEEP computation is satisfied by an ongoing KEEP_AND_REUPLOAD one, but not the other way around, since KEEP wouldn't reupload the blobs that the remote cache lost. Both policies share a key, and KEEP_AND_REUPLOAD relied on executeUnconditionally to stay off a KEEP computation. That dropped the ongoing execution and started a new one in two separate map operations, so a KEEP computation installed by a concurrent caller in between was joined after all and the reupload silently didn't happen. Also fix a caller that cancels its future releasing its reference to the shared execution twice: IndividuallyCancelableFuture completed itself with setFuture, which cancels its argument when it has already been canceled. The outcome is now copied over manually. The stray release is inert today because the execution has left the map by the time it lands, but the invariant is load-bearing for any change that keeps completed executions around. Claude-Session: https://claude.ai/code/session_014jwpqcsMgZBr1aAxuKCDax
A DISCARD computation only needs the root digest, which is known as soon as the tree has been built, but the future of an uploading computation only completes after ensureInputsPresent has pushed the subtree's blobs to the CAS. Joining one therefore traded a local tree build for a wait on the network, and it did so on the path that gates whether an action runs at all. Under dynamic execution, the case it was written for, it coupled the local branch's cache check to the remote branch's upload - precisely the dependency that dynamic execution exists to avoid. Uploading computations are kept under a separate cache key, so dropping the explicit cross-key lookup is all that is needed. It was the only caller of TaskDeduplicator.maybeJoinExecution, which is removed along with it. Claude-Session: https://claude.ai/code/session_014jwpqcsMgZBr1aAxuKCDax
A KEEP_AND_REUPLOAD computation is satisfied by an ongoing computation with the same policy, but only if that computation started after this one discovered that the remote cache had lost the blobs. An earlier reupload may already have uploaded them before they were lost, in which case joining it doesn't restore anything and the retry fails with the same CacheNotFoundException. executeUnconditionally used to provide this implicitly: it dropped the ongoing computation whatever its policy, so a reupload never joined one that had started earlier. Expressing the requirement as a predicate over the blob policy alone lost that, since KEEP_AND_REUPLOAD accepts KEEP_AND_REUPLOAD. The in-flight attributes therefore also carry a sequence number, claimed before the computation is registered, so that every computation that is already ongoing has a lower one than the call that is looking at it. Only reuploads compare it; a weaker request ignores it and keeps joining any computation that retains enough. Claude-Session: https://claude.ai/code/session_014jwpqcsMgZBr1aAxuKCDax
fmeum
force-pushed
the
claude/task-deduplicator-can-join
branch
from
July 29, 2026 14:29
db1a3e8 to
71c3e08
Compare
fmeum
commented
Jul 29, 2026
fmeum
marked this pull request as ready for review
July 29, 2026 15:08
github-actions
Bot
added
team-Remote-Exec
Collaborator
Author
|
@bazel-io fork 9.3.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
TaskDeduplicatordeduplicated executions by key alone, so callers with different requirements either shared an execution that didn't satisfy all of them or had to be separated into different keys, which also gives up sharing in the direction where it is safe.Every execution is now started with caller-provided attributes and every caller supplies a predicate over them, so the join relation can be asymmetric: a caller that needs little joins an execution that produces more, while a caller that needs more starts its own and takes over as the execution that subsequent callers see. The decision is made inside a single
ConcurrentHashMap.compute, which also lets the retry loop around a concurrently canceled execution go away.executeIfNewandexecuteUnconditionallyare replaced by a singleexecute.maybeJoinExecutionis removed as its only use is questionable: Having aDISCARDbuild wait for aKEEPbuild is not a clear performance win since the latter may issue network calls.This also fixes a caller that cancels its future releasing its reference to the shared execution twice:
IndividuallyCancelableFuturecompleted itself withsetFuture, which cancels its argument when it has already been canceled. This was harmless as used today, but not obvious without referring to Guava code and potentially problematic for future changes.Motivation
Simplify the interface of
TaskDeduplicatorand make its inner working more obvious. At the same time, make the reuse of ongoing executions more explicit and flexible inMerkleTreeComputer.Build API Changes
No
Checklist
Release Notes
RELNOTES: None