Skip to content

task_router: route by target package#11953

Open
sluongng wants to merge 1 commit intomasterfrom
sluongng/task-router-target-label-affinity
Open

task_router: route by target package#11953
sluongng wants to merge 1 commit intomasterfrom
sluongng/task-router-target-label-affinity

Conversation

@sluongng
Copy link
Copy Markdown
Contributor

@sluongng sluongng commented Apr 22, 2026

Review feedback asked the affinity experiment to group by package because
the full target label still splits sibling targets in the same package.
Use the Bazel package label instead, so related actions such as
//foo/bar:foo_lib and //foo/bar:foo_test can share warmed executors.

Keep the existing first-output fallback when request metadata is missing.
Cover selected-org, control-org, same-package, and different-package
behavior in task_router tests.

Testing:

  • ./buildfix.sh
  • bazel test --config=remote-minimal //enterprise/server/scheduling/task_router:task_router_test

@iain-macdonald
Copy link
Copy Markdown
Contributor

I think this is an interesting idea and I'd be down to try it out. I see this PR is marked as a "draft" -- are you still working on it, or should I take a look?

@vanja-p
Copy link
Copy Markdown
Contributor

vanja-p commented Apr 24, 2026

BTW, if we are going to use the target label, maybe the package label would be even better.

@sluongng sluongng marked this pull request as ready for review April 25, 2026 02:20
Review feedback asked the affinity experiment to group by package
because the full target label still splits sibling targets in the
same package. Use the Bazel package label instead, so related
actions such as //foo/bar:foo_lib and //foo/bar:foo_test can share
warmed executors.

Keep the existing first-output fallback when request metadata is
missing. Cover selected-org, control-org, same-package, and
different-package behavior in task_router tests.
@sluongng sluongng force-pushed the sluongng/task-router-target-label-affinity branch from 1e4d537 to 61afdd3 Compare April 25, 2026 02:58
@sluongng sluongng changed the title task_router: add target-label routing experiment task_router: route by target package Apr 25, 2026
Copy link
Copy Markdown
Member

@bduffany bduffany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

It might be good to also plumb this through to ExecutionTask.experiments (e.g. have RankNodes return RoutingResult instead of a list, and return experiment details) but that's somewhat of a more involved change, and maybe not a blocker

// Preferred node limit for tasks using [persistentWorkerRouter].
persistentWorkerRouterPreferredNodeLimit = 128

affinityRouterUseTargetLabelExperiment = "remote_execution.affinity_router_use_target_label"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
affinityRouterUseTargetLabelExperiment = "remote_execution.affinity_router_use_target_label"
affinityRouterUseTargetPackageExperiment = "remote_execution.affinity_router_use_target_package"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants