scheduler.node_resource_limit is silently unenforced for most nodes when the scheduler runs more than one replica

[dushulin]

scheduler.node_resource_limit is silently unenforced for most nodes when the scheduler runs more than one replica

Summary

FilterByResourceLimit decides admission from per-replica in-memory heartbeat
state, 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 zero
errors.

Mechanism

Three facts combine:

1. The candidate set is global. Schedule builds candidates from
   nodes.Snapshot(false) (services/scheduler/internal/service.go:86), which
   reads AtomicNodeRegistry.nodesByID. In kubernetes discovery mode that map
   is populated from an EndpointSlice watch, so every replica knows every node
   and may schedule onto any of them.

2. The load metrics are per-replica. The snapshot attached to each candidate
   comes from PeekObserved (service.go:91), backed by
   AtomicNodeRegistry.observed (node_registry.go:49) — a plain in-process map
   written only by Heartbeat. Configuring scheduler.redis_addr does not help:
   Redis is used for sandbox-to-node bindings only, never for observed.

3. 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.

FilterByResourceLimit then fails open on anything it cannot evaluate
(services/scheduler/internal/filter.go:16-19):

if n.Snapshot == nil {
    // No heartbeat yet — cannot evaluate limits; keep the node.
    result = append(result, n)
    continue
}

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

1. Deploy the scheduler with discovery.mode: kubernetes and 2+ replicas.
2. Set a small ceiling so it is cheap to observe:

   { "scheduler": { "node_resource_limit": { "max_sandbox_count": 10 } } }

3. Create sandboxes through the gateway until well past nodes x 10.
4. Read each runtime node's own GET /nodes (per-node ground truth). Do not
   use the gateway's aggregated /nodes — it answers from whichever replica that
   gateway 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 available once every
node is at the ceiling. Actual: creates keep succeeding and nodes climb past it.

Observed behaviour

8 nodes, 3 replicas, max_sandbox_count set to 10 (a small value so ~100
sandboxes suffice; the code path is identical at any value).

run	result
200 sandboxes, full speed, through all gateways	8/8 nodes over the ceiling, worst 30 = 3.0x, 0 rejections, 0 errors
120 sandboxes, full speed, through one gateway pinned to a replica observing 4 nodes	8/8 over, worst 16 = 1.6x
96 sandboxes, paced (batches of 8, 12 s apart — longer than the 5 s heartbeat interval)	the 4 observed nodes froze at 11; the 4 unobserved nodes climbed to 13

The paced run separates two distinct effects:

• For nodes the deciding replica does observe, enforcement is exactly correct.
  11 is the expected first blocked value because withinLimit compares strictly
  greater-than, so a node reporting exactly the ceiling is still admitted.
• For nodes it does not observe, there is no enforcement at all. This is the
  actual defect; heartbeat staleness only adds burst-sized overshoot on top.

Supporting observations:

• The heartbeat split across the 3 replicas was 6/0/2, then 0/4/4 after a
  restart — measured via agentenv_scheduler_observed_nodes{status="ready"}.
• 2 of the 3 gateway pods were pinned to a replica observing zero nodes, so
  roughly two thirds of scheduling traffic had no ceiling applied whatsoever.
  This is why the unpaced run reached 3.0x.
• Dropping to a single replica fixes it: that replica observes 8/8 nodes and
  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

1. Document single-replica as a requirement for resource-aware scheduling.
   The docs currently recommend a single replica only when using the in-memory
   binding store; nothing states that node_resource_limit also depends on it.
   This is the zero-code fix and would at least stop the silent failure.
2. Share observed state (e.g. Redis, alongside bindings). Restores hard
   ceilings with multiple replicas, at the cost of putting a network read on the
   scheduling path.
3. Leader-elected single active scheduler, non-leaders idle — the
   kube-scheduler shape. Preserves the current in-memory fast path and gives HA
   without sharding node state.
4. Node-side admission. Have the runtime refuse a create when it is already
   at its own ceiling and let the caller retry elsewhere — the kubelet
   --max-pods shape. This is a hard cap regardless of replica count and is
   immune 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

• Unknown keys are silently dropped. The scheduler config is decoded with a
  plain json.Unmarshal and no DisallowUnknownFields, so a misspelled key such
  as max_sandbox_kount degrades to "no limit for that dimension" with no
  warning.
• The effective limit is not observable. Nothing logs the decoded
  node_resource_limit at startup, so combined with the above there is no way
  for 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 listening startup line locally; glad to upstream it.

Also, the bundled Helm chart has no passthrough for this setting — values.yaml
exposes the other scheduler.* knobs but the ConfigMap template never renders
node_resource_limit, so a chart-based deployment cannot set it at all without
overriding the whole config blob. Can include that in the same PR.

[yingdi-shan]

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]

Thanks! I did check services/README.md before filing — I think the Redis address solves a different problem than the one reported here.

scheduler.redis_addr switches the sandbox-to-node binding store to Redis. At main (6e88ae2), RedisBindingStore only exposes Get / Record / ReconcileNode and writes two key families (bindingKey, nodeKey) — bindings and the per-node sandbox-ID index. It never stores heartbeat snapshots.

The state node_resource_limit reads is separate and still per-replica in-memory:

• AtomicNodeRegistry.observed is a plain map[string]observedNodeRecord, not Redis-backed.
• FilterByResourceLimit reads it via PeekObserved and keeps any node whose Snapshot == nil — the documented fail-open.

Meanwhile nodesByID comes 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_addr already configured (SCHEDULER_REDIS_ADDR set, Redis-backed bindings), 3 replicas, max_sandbox_count: 10:

• Heartbeats split arbitrarily and re-randomize on restart: observed 6/0/2, then 0/4/4 after a roll.
• 200 sandboxes through the LB VIP → 8/8 nodes over the ceiling, worst node 30 (3.0x), zero rejections.
• A paced run (batch 8, 12s settle > the 5s heartbeat interval) separated the two effects: the 4 nodes observed by the deciding replica froze at 11 (correct — the filter is strictly greater-than), while the 4 unobserved nodes climbed to 13.

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]

Thanks. We need to add --query-only for non-primary schedulers. The current HA mechanism and docs for the scheduler does need improvement.