enable per-physical-GPU replica counts in time-slicing and MPS#1787
Open
jonathan-meiri wants to merge 1 commit into
Open
enable per-physical-GPU replica counts in time-slicing and MPS#1787jonathan-meiri wants to merge 1 commit into
jonathan-meiri wants to merge 1 commit into
Conversation
Author
|
Contributing on behalf of @runatom-ai. |
jonathan-meiri
changed the title
jonathan-meiri
changed the title
jonathan-meiri
pushed a commit
to jonathan-meiri/k8s-device-plugin
that referenced
this pull request
May 19, 2026
TDD red phase for an issue independent of NVIDIA#1787. Setup: a node with two physical GPUs of equal advertised replica counts, where one slot on the "second" GPU has already been allocated to another pod. A new pod requests two more slots. The function name and docstring of distributedAlloc promise an even spread, but today the function deterministically picks both of the new pod's slots from the GPU with the most remaining replicas — leaving the other physical GPU's available slot untouched. The bug is in the sort tie-break. After the first pick the per-GPU 'used' counts tie across the candidates, and sort.Slice is unstable, so the next iteration ends up picking the next slot on the GPU we just picked from rather than rotating to the sibling GPU that still has capacity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com> Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
|
This can be extremely useful, is there any way I can help advance this? |
jonathan-meiri
force-pushed
the
heterogeneous-time-slicing-replicas
branch
from
July 14, 2026 13:24
eb475d6 to
c447e8b
Compare
Currently DisableResourceNamingInConfig silently strips per-entry
Rename and any non-Devices.All selection from Sharing.TimeSlicing.Resources
and Sharing.MPS.Resources, collapsing heterogeneous configs into a single
homogeneous resource. Operators cannot configure different replica counts
for different physical GPUs on the same node, even though the device-map
construction in internal/rm/device_map.go (getIDsOfDevicesToReplicate)
already handles Devices.All / Devices.Count / Devices.List correctly.
Replace disableResoureRenaming with applyDefaults, which fills defaults
for unset fields but leaves explicit user configuration intact. Auto-
rename when renameByDefault is true is preserved; Devices.All=true is
still the default when Devices is unset. The two "not yet supported in
the config" warnings go away.
Once devices selection is honored, the MPS daemon must only touch the
physical GPUs it is actually sharing. Previously the daemon iterated
every device in the resource manager (including unreplicated ones that
share the same resource name), which meant setComputeMode set
EXCLUSIVE_PROCESS on GPUs that were not part of the MPS-shared subset,
breaking their normal usage. The same over-reach affected
perDevicePinnedDeviceMemoryLimits and activeThreadPercentage. Introduce
a Daemon.sharedDevices() helper that returns only the annotated
(replicated) subset of the daemon's devices and route the three
call sites through it.
Tests cover:
- api/config/v1: TimeSlicing and MPS both preserve per-entry Rename
and Devices.List across DisableResourceNamingInConfig.
- cmd/mps-control-daemon/mps: sharedDevices() excludes unreplicated
devices; perDevicePinnedDeviceMemoryLimits and
activeThreadPercentage compute using the shared subset only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: runatom-ai <258621014+runatom-ai@users.noreply.github.com>
Signed-off-by: Jonathan Meiri <33288957+Meiri28@users.noreply.github.com>
jonathan-meiri
force-pushed
the
heterogeneous-time-slicing-replicas
branch
from
July 15, 2026 12:31
c447e8b to
3db6617
Compare
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
3 participants
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.
Summary
Enable per-physical-GPU replica counts in
sharing.timeSlicingandsharing.mpsconfig. See #1786 for the design context (prior art, what this does and doesn't try to do, MPS vs time-slicing semantics).Contributed by @Meiri28 on behalf of @runatom-ai.
Closes #1786
Changes
api/config/v1/replicas.go: replacedisableResoureRenamingwithapplyDefaults. The new helper still fills the existing defaults (auto-rename whenrenameByDefault: trueandRenameis unset; defaultDevices.All = truewhen no selector is set) but no longer strips user-suppliedRenameorDevices.List/Devices.Count. The two "not yet supported in the config" warnings go away.api/config/v1/config.go: updateDisableResourceNamingInConfigto call the new helper. Function name and public signature unchanged.internal/rm/allocate.go: indistributedAlloc, change the sort key frommin(total - available)tomax(available). The two orderings are equivalent in the homogeneous case, but the previous key biased toward exhausting smaller physical GPUs first when replica counts differ. The now-unusedtotalfield and its initialization loop are removed.Device-map construction in
internal/rm/device_map.gois unchanged — it already handlesDevices.All/Devices.Count/Devices.Listcorrectly. Net behavior change is ~50 LoC plus tests.Commits are DCO-signed.