Dale Richardson created YUNIKORN-3356:
-----------------------------------------
Summary: Split the shim's Kubernetes clients by concern (writes /
informers / events) with per-concern rate limits and User-Agents
Key: YUNIKORN-3356
URL: https://issues.apache.org/jira/browse/YUNIKORN-3356
Project: Apache YuniKorn
Issue Type: Improvement
Components: shim - kubernetes
Reporter: Dale Richardson
h2. Summary
The shim builds two general-purpose clientsets whose groupings follow code
structure rather than traffic type: one serves both the informers and the
events sink, the other serves writes, the volume binder, and predicate lookups.
Because each clientset gets its own rate limiter from the same
{{kubernetes.qps}} setting, (a) unrelated traffic classes share failure domains
— an event burst can starve informer watch re-establishment during recovery —
and (b) the effective request ceiling is 2× the configured value, which is
unlikely to be what operators expect.
Proposal: restructure into purpose-built clients — {*}writes/binds{*},
{*}informers{*}, *events* (and later *leader election* if it is ever added) —
each with its own rate limiter, sensible per-concern defaults, and a distinct
User-Agent.
h2. Current state (master, {{pkg/client}} / {{{}pkg/shim{}}})
* Bootstrap client ({{{}pkg/client/kubeclient.go:43{}}},
{{{}interfaces.go:58{}}}, called from {{{}bootstrap.go:30{}}}): two ConfigMap
GETs at startup. Sets no QPS, so no clientset-level limiter is created and it
runs on client-go's rest-layer defaults (5 QPS / 10 burst).
* Clientset 2 ({{{}pkg/shim/scheduler.go:67{}}}): serves *both* the
cluster-wide SharedInformerFactory ({{{}scheduler.go:70{}}}) *and* the
events/v1 broadcaster sink ({{{}scheduler.go:84-85{}}}).
* Clientset 3 ({{{}pkg/client/apifactory.go:93{}}}): serves *all writes*
(Bind/Create/Delete/UpdateStatus), the namespaced ConfigMap informer factory,
the volume binder ({{{}apifactory.go:122{}}}), and the predicate framework
handle.
Consequences:
# *{{kubernetes.qps}} is a per-clientset limit, so the effective ceiling is 2×
the configured value.* Each {{NewKubeClient}} call builds a fresh
{{rest.Config}} ({{{}kubeclient.go:55-69{}}}) and client-go creates a new token
bucket per clientset ({{{}client-go kubernetes/clientset.go{}}},
{{{}NewForConfigAndClient{}}}). An operator setting {{kubernetes.qps: 500}}
expecting to cap the scheduler at 500 req/s actually allows ~1,000. Any further
client splitting silently multiplies this again unless the config surface is
redesigned — so this restructuring and the config semantics must land together.
# *An event flood competes with watch re-establishment on the same token
bucket* (clientset 2). Mass-failure scenarios emit events in bursts precisely
when informers need tokens to reconnect and relist — the worst possible
coupling (see YUNIKORN-3355 for how much recovery correctness depends on timely
informer delivery). Severity scales inversely with the configured QPS: at the
1000/1000 default a mass-failure event burst (thousands of events) delays
informer re-establishment by seconds; at operator-lowered values (e.g. 50,
kube-scheduler parity) the same burst starves the watch path for minutes.
Splitting removes the coupling at every setting.
# {*}Events compete with nothing they should{*}: events are droppable by
design (the events/v1 broadcaster already has a bounded 1000-entry queue with
drop-on-full), yet today they can only be limited by limiting everything else
too.
# No {{UserAgent}} is set on any client (zero occurrences in non-test code),
so apiserver-side attribution (audit logs, per-client debugging) cannot
distinguish the shim's write path from its watch path.
h2. Proposed design
||Client||Serves||Limiter||Config||
|writes|Bind, Create/Delete, status updates, volume binder|token
bucket|{{kubernetes.qps}} / {{kubernetes.burst}} — {*}semantics preserved{*}:
this is the knob that caps how fast YuniKorn acts on the cluster (binds are
~1:1 with QPS tokens; measured 52 vs 1,170 binds/s at 50 vs 1000)|
|informers|all SharedInformerFactory watch/list traffic|*none* (unlimited)|—
steady-state request rate is intrinsically tiny (watches are long-lived; the
limiter charges per request, not per event); relist bursts are better governed
server-side by APF than by a client limiter that would slow recovery|
|events|events/v1 broadcaster sink|token bucket|new {{kubernetes.eventQPS}} /
{{kubernetes.eventBurst}} (suggested default 200/400; kubelet precedent
{{{}eventRecordQPS{}}}). Bounded loss under storm is acceptable and now
*possible without capping binds*|
|bootstrap|2 startup ConfigMap GETs|n/a|fold into the writes client; the
dedicated 5/10-QPS clientset is vestigial|
Each client sets a distinct {{UserAgent}} ({{{}yunikorn-scheduler/writes{}}},
{{{}/informers{}}}, {{{}/events{}}}) for apiserver-side attribution and cleaner
{{rest_client_*}} metrics if/when client-go metrics are registered.
h2. Interactions
* {*}Config semantics{*}: this issue narrows {{kubernetes.qps}} from "per
clientset, effectively ×2" to "write path only". Release notes must state both
the old effective behaviour and the new meaning. The documentation work for QPS
guidance should land as part of this change (one table: client / purpose / knob
/ default) rather than separately.
* {*}Hot-reload{*}: with per-concern defaults there is little reason left to
make QPS hot-reloadable; recommend dropping that idea rather than building
config-rebuild machinery for multiple clients. Server-side APF is the correct
dynamic control.
* {*}APF (future, optional phase 2){*}: APF distinguishes flows by {*}user{*},
so split clients on one ServiceAccount still share one queue position. Running
the events client under a separate identity would allow events to be placed in
a cheap priority level while the write path keeps a protected one. Not required
for this issue, but the client boundary drawn here is what makes it possible
later — worth keeping in mind during review.
* {*}Write dispatcher (KEP-5229-style, future){*}: a bounded write-dispatch
worker pool would sit naturally on the writes client; clean separation here
simplifies that later work.
h2. Risks / compatibility
* Connection count rises by ~2 HTTP/2 connections — negligible.
* Operators who relied (knowingly or not) on the 2× effective ceiling will see
the informer path become unlimited and the write path unchanged; net behaviour
change in normal operation is nil, but it should be called out.
* Event delivery under sustained storm becomes explicitly bounded (today it is
implicitly bounded by the broadcaster's 1000-entry drop-on-full queue anyway).
h2. Validation
* Unit: config plumbing (defaults, back-compat of {{{}kubernetes.qps{}}}).
* e2e: mass-failure event storm concurrent with a bind burst — assert bind
throughput is unaffected by event-path saturation (today they share a bucket);
assert informer watch re-establishment is unaffected by an event backlog. A
kind+KWOK harness that produces both conditions exists from the API-server load
characterisation work and can be contributed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
