mirror of
https://github.com/kvcache-ai/AgentENV.git
synced 2026-11-05 01:17:36 +00:00
[GH-ISSUE #191] scheduler.node_resource_limit is silently unenforced for most nodes when the scheduler runs more than one replica #62
Labels
No labels
bug
documentation
enhancement
enhancement
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
kvcache-ai/AgentENV#62
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @dushulin on GitHub (Aug 19, 2026).
Original GitHub issue: https://github.com/kvcache-ai/AgentENV/issues/191
scheduler.node_resource_limitis silently unenforced for most nodes when the scheduler runs more than one replicaSummary
FilterByResourceLimitdecides admission from per-replica in-memory heartbeatstate, but the candidate node set is global. Each runtime node's heartbeat is
pinned to exactly one scheduler replica, so any given replica has snapshots for
only a subset of the nodes it can schedule onto — and the filter keeps nodes it
has no snapshot for. With more than one replica the configured per-node
ceilings therefore apply to a shifting minority of nodes, with no error, no
rejection, and no log line indicating a limit was skipped.
Measured on an 8-node cluster with 3 scheduler replicas: every node exceeded
max_sandbox_count, worst case 3.0x over, with zero rejections and zeroerrors.
Mechanism
Three facts combine:
The candidate set is global.
Schedulebuilds candidates fromnodes.Snapshot(false)(services/scheduler/internal/service.go:86), whichreads
AtomicNodeRegistry.nodesByID. Inkubernetesdiscovery mode that mapis populated from an EndpointSlice watch, so every replica knows every node
and may schedule onto any of them.
The load metrics are per-replica. The snapshot attached to each candidate
comes from
PeekObserved(service.go:91), backed byAtomicNodeRegistry.observed(node_registry.go:49) — a plain in-process mapwritten only by
Heartbeat. Configuringscheduler.redis_addrdoes not help:Redis is used for sandbox-to-node bindings only, never for
observed.Each node heartbeats to exactly one replica. The runtime's reporter opens
a single lazily-connected gRPC channel to the scheduler Service and reuses it,
so every node's heartbeats land on whichever replica that one connection
resolved to. The distribution is arbitrary and re-randomizes whenever a
replica restarts.
FilterByResourceLimitthen fails open on anything it cannot evaluate(
services/scheduler/internal/filter.go:16-19):That branch is correct in isolation — a node that has genuinely not reported yet
should not be excluded — but under sharded heartbeats it is the common case
rather than the rare one, so the ceiling quietly evaporates for most of the
fleet.
The same sharding also affects gateways: each gateway pod holds one channel to
one replica, so which replica (and therefore which subset of enforced nodes) a
request is decided by depends on which gateway pod received it.
Reproduction
discovery.mode: kubernetesand 2+ replicas.nodes x 10.GET /nodes(per-node ground truth). Do notuse the gateway's aggregated
/nodes— it answers from whichever replica thatgateway is pinned to and so reports only that replica's observed subset, which
is the very thing under test.
Expected: creates start failing with
Unavailable: no nodes availableonce everynode is at the ceiling. Actual: creates keep succeeding and nodes climb past it.
Observed behaviour
8 nodes, 3 replicas,
max_sandbox_countset to 10 (a small value so ~100sandboxes suffice; the code path is identical at any value).
The paced run separates two distinct effects:
11 is the expected first blocked value because
withinLimitcompares strictlygreater-than, so a node reporting exactly the ceiling is still admitted.
actual defect; heartbeat staleness only adds burst-sized overshoot on top.
Supporting observations:
6/0/2, then0/4/4after arestart — measured via
agentenv_scheduler_observed_nodes{status="ready"}.roughly two thirds of scheduling traffic had no ceiling applied whatsoever.
This is why the unpaced run reached 3.0x.
the ceiling holds. This is what we deployed, at the cost of control-plane
availability during scheduler restarts.
Fail-closed is not a fix
Excluding nodes without a snapshot would be worse: a replica observing one node
would reject the other seven and pile every sandbox onto that one node. A
correct fix needs the deciding replica to have the state, not to guess
pessimistically without it.
Possible directions
The docs currently recommend a single replica only when using the in-memory
binding store; nothing states that
node_resource_limitalso depends on it.This is the zero-code fix and would at least stop the silent failure.
ceilings with multiple replicas, at the cost of putting a network read on the
scheduling path.
kube-scheduler shape. Preserves the current in-memory fast path and gives HA
without sharding node state.
at its own ceiling and let the caller retry elsewhere — the kubelet
--max-podsshape. This is a hard cap regardless of replica count and isimmune to heartbeat staleness, but needs a retry path in the gateway so a
caller does not see a failure while other nodes still have room.
Happy to send a PR for whichever direction maintainers prefer, (1) included.
Two smaller adjacent findings
plain
json.Unmarshaland noDisallowUnknownFields, so a misspelled key suchas
max_sandbox_kountdegrades to "no limit for that dimension" with nowarning.
node_resource_limitat startup, so combined with the above there is no wayfor an operator to confirm from outside the process that the limit they wrote
is the limit in force. We added the decoded value to the existing
scheduler gRPC server listeningstartup line locally; glad to upstream it.Also, the bundled Helm chart has no passthrough for this setting —
values.yamlexposes the other
scheduler.*knobs but the ConfigMap template never rendersnode_resource_limit, so a chart-based deployment cannot set it at all withoutoverriding the whole config blob. Can include that in the same PR.
@yingdi-shan commented on GitHub (Aug 19, 2026):
To enable multiple scheduler replicas, please refer to services/README.md, which explains how to provide a Redis address. We're planning to add better HA support in the next few weeks.
@dushulin commented on GitHub (Aug 19, 2026):
Thanks! I did check
services/README.mdbefore filing — I think the Redis address solves a different problem than the one reported here.scheduler.redis_addrswitches the sandbox-to-node binding store to Redis. Atmain(6e88ae2),RedisBindingStoreonly exposesGet/Record/ReconcileNodeand writes two key families (bindingKey,nodeKey) — bindings and the per-node sandbox-ID index. It never stores heartbeat snapshots.The state
node_resource_limitreads is separate and still per-replica in-memory:AtomicNodeRegistry.observedis a plainmap[string]observedNodeRecord, not Redis-backed.FilterByResourceLimitreads it viaPeekObservedand keeps any node whoseSnapshot == nil— the documented fail-open.Meanwhile
nodesByIDcomes from the EndpointSlice watch, so every replica sees all nodes and may schedule onto any of them, while each node's heartbeats land on exactly one replica. A replica that never received node X's heartbeat treats X as unlimited.Importantly, the numbers in the report were measured with
scheduler.redis_addralready configured (SCHEDULER_REDIS_ADDRset, Redis-backed bindings), 3 replicas,max_sandbox_count: 10:So Redis-backed bindings are orthogonal: the ceiling is enforced exactly right for the observed subset and fails open for the rest. Good to hear HA work is coming — sharing
observed, or having the filter consult a shared store, would close this specific gap. Happy to test a branch. Meanwhile we're staying on a single replica in production.Minor docs suggestion: the "Node resource limit" section could note that the ceiling only covers heartbeats the answering replica has observed, so it is effectively single-replica-only today.
@yingdi-shan commented on GitHub (Aug 19, 2026):
Thanks. We need to add --query-only for non-primary schedulers. The current HA mechanism and docs for the scheduler does need improvement.
@dushulin commented on GitHub (Sep 7, 2026):
hi, @yingdi-shan Do we have a plan to fix this issue? we want to scale our sandbox count, so scheuler HA and multi replica is very important
@yingdi-shan commented on GitHub (Sep 7, 2026):
Yes, please check issue #259.
@NickNYU commented on GitHub (Oct 5, 2026):
Hi @dushulin,
Thanks for the detailed write-up, the repro data made this easy to reason about. Leaving an observation on how #341 (scheduler HA via Lease leader election, from #259) relates to this issue.
Where things stand
#341 lands direction 3 from your "Possible directions", with one difference: non-leaders are not fully idle.
Schedule,RecordAssignment,Heartbeat, sandbox events, P2P writes) are served only by the leader. Standbys returnUnavailable("not the leader").LookupNode,GetNode) are served by standbys from the shared Redis bindings whenscheduler.redis_addris set.For this issue only (1) and (3) matter:
Schedule, and every heartbeat lands on the leaderWhy heartbeats converge on the leader:
HeartbeatwithUnavailable, and the readiness gate keeps them out of Service endpoints, so new connections only reach the leader.filter.gono longer sees "sharded away" nodes, only genuinely unreported ones.What #341 does not cover from this issue:
DisallowUnknownFieldson the scheduler config, and logging the decodednode_resource_limitat startup.node_resource_limit.Your offer to upstream those still looks like the right remaining piece.
One caveat on the standby reads, for completeness: since Service endpoints hold only the leader, that read path is reachable today only over connections pinned before a failover. Turning it into a real read path needs a second, ungated read Service plus
gateway.query_only_scheduler_addrpointed at it. Manifests only, the gateway client hook already exists. Not required for this issue either way.One confirmation:
@yingdi-shan does this match your read: #191 stays open until the repro (multi-replica, small
max_sandbox_count) is re-run againstdeploy/k8s/overlays/ha, and the adjacent findings go in a separate PR?Thanks