Skip to content

fix(planner): resolve aggregate workers by component type (cherry-pick #11578) - #12779

Merged
pvijayakrish merged 2 commits into
release/1.4.0from
dagil-nvidia/cp-11578-planner-aggregate-workers
Aug 6, 2026
Merged

fix(planner): resolve aggregate workers by component type (cherry-pick #11578)#12779
pvijayakrish merged 2 commits into
release/1.4.0from
dagil-nvidia/cp-11578-planner-aggregate-workers

Conversation

@dagil-nvidia

@dagil-nvidia dagil-nvidia commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Clean cherry-pick of #11578 onto release/1.4.0 (no conflicts, 2 files, +58).

  • Resolve aggregate planner workers by their DGD component type instead of backend-specific names.
  • Treat the sole generic type: worker component as the planner's decode component, regardless of its name.
  • Keeps the existing explicit-name fallback when multiple generic workers make the type ambiguous.

Fixes the planner bug reported in #12384 for the 1.4.0 release. Original PR merged to main on 2026-07-30 (d66670faaa).

🤖 Generated with Claude Code


Open in Devin Review

Signed-off-by: WEI CHENG CHIU <waynehacking8@gmail.com>
(cherry picked from commit d66670f)
Signed-off-by: Dan Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia requested a review from a team as a code owner August 6, 2026 22:07

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +213 to +220
if not matching_components and component_type == SubComponentType.DECODE:
generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Explicitly configured decode worker can be ignored in favour of a differently-named worker

The lone generic worker is picked as the decode worker (generic_workers check at components/src/dynamo/planner/monitoring/dgd_services.py:213-220) before the explicitly configured worker name is even looked at, so the planner can scale a worker the operator never asked it to scale.
Impact: In a deployment that names its decode worker explicitly while another worker is only generically labelled, the planner resizes the wrong worker group, breaking autoscaling for the intended one.

Precedence between type-based fallback and explicit component name

In get_component_from_type_or_name, the new block runs before the component_name in components branch (components/src/dynamo/planner/monitoring/dgd_services.py:222-226). Consider a v1beta1 DGD with component A of type: worker (actually prefill) and component B with no type field, where the caller passes component_name="B" for SubComponentType.DECODE. No component has type: decode, so matching_components is empty; the fallback finds exactly one generic worker (A) and selects it, and the explicit-name branch is skipped. Previously B would have been selected via _can_use_explicit_component_name (which explicitly permits an empty type, see components/src/dynamo/planner/monitoring/dgd_services.py:88-96). All callers that pass decode_component_name (components/src/dynamo/planner/connectors/kubernetes.py:236-240, :660-664, :700-704) are affected, including replica updates.

A safer ordering is to try the explicit name first and only use the sole-generic-worker fallback when no usable explicit name is given.

Suggested change
if not matching_components and component_type == SubComponentType.DECODE:
generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers
if (
not matching_components
and component_type == SubComponentType.DECODE
and component_name not in components
):
generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc1c966a97

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

component_names = [name for name, _ in matching_components]
raise DuplicateSubComponentError(component_type.value, component_names)

if not matching_components and component_type == SubComponentType.DECODE:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Sign the commit with DCO

The reviewed commit has no Signed-off-by: trailer, so it violates the repository requirement to sign every commit with git commit -s and will not satisfy DCO validation. Recreate this commit with the required sign-off before merging.

AGENTS.md reference: AGENTS.md:L131-L138

Useful? React with 👍 / 👎.

@dagil-nvidia

Copy link
Copy Markdown
Collaborator Author

/ok to test fc1c966

@pvijayakrish
pvijayakrish merged commit 3a87a33 into release/1.4.0 Aug 6, 2026
2 checks passed
@pvijayakrish
pvijayakrish deleted the dagil-nvidia/cp-11578-planner-aggregate-workers branch August 6, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants