Experiment/reservation race pending - #1
Draft
cgetzen wants to merge 7 commits into
Draft
Conversation
Meshy fork: let a custom/external scheduler (e.g. slurm-bridge) place the whole-GPU reservation pod so Slurm accounts the whole GPU, while KAI packs fractions onto it. Validated e2e on slurm-staging (Tesla T4): reservation placed by slurm-bridge as a DRA extended-resource request, 2x0.5 fractions shared, no double-booking. - Reservation pod can request GPUs via the DRA extended resource (deviceclass.resource.kubernetes.io/gpu.nvidia.com) instead of the classic nvidia.com/gpu device-plugin resource. - New Config knobs threaded operator -> binder args: reservationSchedulerName, reservationGpuResourceName, reservationPodAnnotations. - External mode: create the reservation pod and wait for the external scheduler to place it (follow-the-reservation), with a node-divergence guard for the v1 static single-GPU-node pool. - Wire --resource-reservation-allocation-timeout through options/app. - Relax the HasDRAGPUs fractional guard to reject only DRA-ONLY nodes; a node that also advertises device-plugin GPUs (+ nvidia.com/gpu.memory) has the signals KAI's sharing accounting needs, so fractions work there even though the reservation is held via DRA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix A for the reservation-race work: the root cause of the frac-pod create->pend->reap churn on a "both" node (device plugin AND DRA exposing the same physical GPUs). - populateDRAGPUs added DRA ResourceSlice GPUs on TOP of the device-plugin allocatable, double-counting one physical T4 as two. The extra phantom idle GPU made the scheduler keep opening new whole-GPU groups for an already-unschedulable fraction, spawning reservation pods that slurm-bridge could never place. Now add only max(0, DRA - device-plugin): the node's true GPU count is max(device-plugin, DRA), since both describe the same hardware. - Fix the "fractional not supported on DRA-only nodes" guard: it tested AllocatableVector[GPU] <= 0, but AddDRAGPUs merges DRA GPUs into that vector, so it was never <= 0 on a DRA node and the guard silently no-op'd (broke the upstream TestPredicateByNodeResourcesType_SharedGPU_DRANode). Capture the device-plugin GPU count at construction (new NodeInfo.DevicePluginGPUs, set before AddDRAGPUs) and gate on that instead. Tests: both-node + excess-DRA cases in cluster_info; both-node accept case in node_info; the previously-broken DRA-only reject cases pass again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Design-2 foundation for the reservation race: today a reservation pod that does not allocate within --resource-reservation-allocation-timeout (40s) is deleted and the BindRequest fails+rolls back, so an externally-scheduled reservation (slurm-bridge) that is merely queued in Slurm gets churned. Now, when the reservation pod is still Pending at timeout (vs failed/gone), the binder PARKS it: - waitForGPUReservationPodAllocation returns a distinct pendingGpuIndicator; timeoutAllocationIndicator inspects the pod phase to decide park vs reap. - createGPUReservationPodAndGetIndex does not delete a parked reservation and returns ErrReservationPending (wrapped, so errors.Is works up the stack). - The BindRequest reconciler treats ErrReservationPending as "retry later": no Rollback (keeps the fraction's GPU-group binding), no terminal phase, no failed-attempt increment; just RequeueAfter to poll for placement. Teardown is already handled: a parked reservation is protected from GC while its BindRequest is non-terminal (hasActiveBindRequestsForGpuGroup); once the fraction binds elsewhere (packs onto a freed GPU) and the BindRequest goes terminal, the reservation sync deletes it. Unit test covers the park-vs-reap decision. Note: this parks reservations the scheduler already proposed; making the scheduler reserve-ahead for an unschedulable fraction on a full node (so a reservation is enqueued to Slurm speculatively) is the remaining scheduler-side piece. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… race)
Design-2 completion: a decoupled reconciler that reserves a whole GPU ahead for
a fraction that can't yet pack, WITHOUT the KAI scheduler having to decide
cluster fullness. It composes with the parking primitive and Fix A's pack-on-free
rather than modifying core scheduler fit logic.
For a GPU-fraction pod that has been Pending + unschedulable past a short delay:
- EnsureReservation creates ONE parked whole-GPU reservation (external/slurm-bridge,
no nodeName) and labels the fraction with the reservation's stable GPU group, so
the existing reservation sync keeps it alive and reaps it once the fraction
settles elsewhere.
- The fraction is deliberately left Pending, so the KAI scheduler keeps trying to
pack it onto a GPU that frees up. Whichever wins the race:
* scheduler packs it onto a freed reserved GPU -> its group label flips ->
reserve-ahead reservation orphaned -> torn down by the sync;
* reserve-ahead reservation gets placed (Slurm preempts/queues) ->
ReservationPlacement reports ready -> we create the BindRequest to bind the
fraction onto it.
- BindRequests are named per-pod, so a scheduler/reconciler race resolves to a
single winner (AlreadyExists -> back off).
New reservation-service methods: EnsureReservation (node-less external create) and
ReservationPlacement (ready + node). Enabled only in external-reservation mode.
Wired into the binder manager. Unit tests cover group stability, the wait-before-
reserve delay, ensure+label, and bind-on-placement. No core scheduler changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…correctness) On-cluster testing (2-GPU staging) surfaced a bug: after the scheduler packed a parked fraction onto a freed GPU, the reserve-ahead reconciler kept running (its cache still saw the pod Pending) and re-labeled the fraction back to the reserve-ahead group, overwriting the binder's real group. That (a) corrupted the scheduler's GPU-group accounting and (b) leaked the parked reservation (its group still looked referenced, so the sync never reaped it). Fix: a BindRequest for the fraction means its placement is decided, so the reconciler stops touching it: - BindRequest binds onto our reserve-ahead group -> reservation won the race; keep it. - BindRequest binds onto a different group (pack won) -> repoint the pod's group label to the real bound group and tear down the now-orphaned reserve-ahead reservation. The ensure/label path only runs while NO BindRequest exists. Validated on a 2x T4 cluster: multi-GPU packing; reserve-ahead parks one reservation (no churn); pack-wins race repoints the label + reaps the reservation; whole-GPU-drain converges to one reservation per GPU with no leak. Slurm shows exactly one gres/gpu job per node; fractions share the correct physical GPU; zero device-plugin GPU consumers (pure DRA, no double-book). New unit tests cover the pack-won teardown and reservation-won keep paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…deleted
Deleting a reservation ("capacity") pod frees the whole GPU in Slurm immediately
(slurm-bridge cancels the GRES job on pod termination) while the fraction
workloads keep running on it — a silent double-booking. Neither a finalizer nor a
grace period can keep the GPU held (kubelet terminates the container on
deletionTimestamp regardless of finalizers; k8s finalizers are unordered so they
can't gate slurm-bridge's cleanup). So instead, cascade the deletion downward.
- Externally-scheduled reservation pods now carry ReservationCascadeFinalizer.
- New ReservationCascadeReconciler watches reservation pods: on deletion it deletes
the GPU-fraction pods sharing the reservation's GPU group, and HOLDS the finalizer
until they are actually gone — so the reservation stays Terminating until the
workloads have drained, then is released. This is the "wait for the other pods to
terminate" behavior, implemented via the finalizer (terminationGracePeriodSeconds
only governs a pod's own containers, not others).
Enabled only in external-reservation mode (where the binder stamps the finalizer),
so vanilla-mode reservation pods are never stranded by a finalizer nothing removes.
Unit tests cover cascade+wait, immediate-release when no fractions, and disabled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…al churn The reservation-cascade controller fired on ANY reservation-pod deletion, including the reservation service's own internal deletes (reap-on-timeout, the node-divergence guard, and syncForPods orphan cleanup). During the bind window the sync can transiently see a group as having "no fraction pod" (the fraction's gpu-group label isn't visible in its informer yet, and the BindRequest is already gone so hasActiveBindRequestsForGpuGroup doesn't protect) and delete the reservation — which the cascade then amplified into deleting the just-bound, healthy fraction workload. Reproduced on-cluster: a fraction pod deleted ~15s after binding. Fix: - Internal deleteReservationPod strips the cascade finalizer before deleting, so a system-initiated deletion does NOT cascade. Only an external delete (which leaves the finalizer intact) triggers the cascade — the intended semantic. - The cascade controller no longer re-adds the finalizer on non-deleting reservations (the binder stamps it at creation); re-adding would race-undo the strip above. Net: internal reservation churn is back to non-lethal (at worst a reservation flap), while `kubectl delete` of a capacity pod still cascades to its fractions. Unit test covers the finalizer strip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Related Issues
Fixes #
Checklist
make changelog(or applied theskip-changeloglabel). Do not editCHANGELOG.mddirectly — pending fragments are folded into it at release time.Breaking Changes
Additional Notes