Skip to content

api/config/v1: keep user-specified rename/devices for shared resources - #1825

Open
jonyhy96 wants to merge 2 commits into
NVIDIA:mainfrom
jonyhy96:feature/perdevice-uuid-timeslicing
Open

api/config/v1: keep user-specified rename/devices for shared resources#1825
jonyhy96 wants to merge 2 commits into
NVIDIA:mainfrom
jonyhy96:feature/perdevice-uuid-timeslicing

Conversation

@jonyhy96

@jonyhy96 jonyhy96 commented Jun 2, 2026

Copy link
Copy Markdown

disableResoureRenaming currently resets the Rename and Devices fields of every entry under sharing.timeSlicing.resources (and sharing.mps.resources) to their defaults. That makes it impossible to:

  • pin a shared resource to a specific subset of GPU UUIDs (per-UUID time-slicing), and
  • expose two resource names on the same node, e.g. nvidia.com/gpu for full cards and nvidia.com/gpu.shared for the time-sliced subset.

Preserve both fields and emit a warning instead, so that user intent is honored. Downstream resource-manager code already supports per-UUID device lists and custom rename targets, so no additional plumbing is required.

@copy-pr-bot

copy-pr-bot Bot commented Jun 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@rajatchopra

Copy link
Copy Markdown
Contributor

@jonyhy96 Can you provide a corresponding unit test case here? Possibly one that shows how two resource names are used within a node.

@jonyhy96
jonyhy96 force-pushed the feature/perdevice-uuid-timeslicing branch from b5d1686 to 85954f6 Compare June 25, 2026 06:41
@jonyhy96

Copy link
Copy Markdown
Author

@rajatchopra Thanks for the review! I've pushed a unit test TestDisableResoureRenamingKeepsUserSpec in api/config/v1/replicas_test.go (commit 85954f6) covering four cases:

  1. Per-UUID rename preserved when renameByDefault=false.

  2. Per-UUID devices list preserved when renameByDefault=true with the default rename target.

  3. Two resource names on a single node — exactly the scenario you asked about: nvidia.com/gpu for full cards and nvidia.com/gpu.shared carrying only a UUID-selected subset (devices: ["GPU-"]). Any GPU on the node that isn't listed stays on the original nvidia.com/gpu resource name, so the two resource names coexist on the same node.

  4. nil receiver no-op for defensive coverage.

All four sub-tests pass locally (go test ./api/config/v1/... is green). PTAL.

@henry118

Copy link
Copy Markdown
Member

Looking at the #490 that introduced the disableResoureRenaming logic, resetting Rename and Devices fields seems intentional to me. Particularly this comment explained the limitation of the MPS implementation.

IIUC MPS requires all GPUs on a node to share the same replica count. This is enforced by disableResoureRenaming which forces Devices.All = true (preventing per-GPU device selection) and clears custom rename fields (preventing multiple resource names).

haoyun added 2 commits July 21, 2026 11:39
For the time-slicing sharing path, preserve user-specified rename and devices fields in ReplicatedResources so that per-UUID time-slicing (where a subset of GPUs on a node is exposed under a renamed shared resource, e.g. nvidia.com/gpu.shared, while the remaining devices stay on nvidia.com/gpu) can be expressed in the config.

The MPS control daemon can only apply a single active_thread_percentage per node, so per-GPU device selection and multiple rename targets remain structurally unsupportable on the MPS path. The MPS branch therefore retains the historical behavior: user-specified rename and devices fields are reset and the original warnings are emitted.

Signed-off-by: haoyun <haoyun.96@bytedance.com>
…d MPS reset

TestDisableResoureRenamingKeepsUserSpec covers the time-slicing path: per-UUID rename and devices fields are preserved (including the case where a single node exposes both nvidia.com/gpu full cards and nvidia.com/gpu.shared per-UUID slices).

TestDisableResoureRenamingMPSStillResets guards the MPS invariant: because the MPS control daemon can only apply a single active_thread_percentage per node, disableResoureRenaming(mps) must keep clearing user-specified rename and forcing Devices.All=true.

Signed-off-by: haoyun <haoyun.96@bytedance.com>
@jonyhy96
jonyhy96 force-pushed the feature/perdevice-uuid-timeslicing branch from 3a7279a to f30be88 Compare July 21, 2026 03:41
@jonyhy96

Copy link
Copy Markdown
Author

@henry118 Good catch — thanks for flagging the MPS constraint. You're right that MPS and time-slicing share this helper but have very different invariants: the MPS control daemon can only apply one active_thread_percentage per node, so per-GPU device selection and multiple rename targets aren't representable on the MPS path, whereas time-slicing has no equivalent per-node limit.

I've narrowed the change so it only affects the time-slicing branch:

  • disableResoureRenaming("mps") still clears user-specified Rename and forces Devices.All=true, with the original warning wording — the MPS invariant is preserved byte-for-byte.

  • Only disableResoureRenaming("timeSlicing") (and any future non-MPS sharing modes) now keeps user-specified rename / devices, so per-UUID time-slicing on a subset of GPUs (e.g. some GPUs on the node stay on nvidia.com/gpu while others are sliced into nvidia.com/gpu.shared) works end-to-end.

  • Added TestDisableResoureRenamingMPSStillResets as a regression test that pins the MPS reset behaviour so this can't drift in the future.

Force-pushed as f30be88. PTAL.

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.

3 participants