Repository navigation
feat(umbp): serve metrics from a masterless node, and summarise offload/load/evict/promote/demote at INFO - #683
Conversation
…tandalone server A UMBP peer measured everything and, without a master, published none of it. MasterClient was the only metric destination in the process: PoolClient's CountMetric returned early when master_client_ was null, PublishComponentMetrics returned early for the same reason, and the component tick it drives was registered only through MasterClient::AddMetricsProvider. The standalone server -- the deployment sglang's direct external linker actually runs -- therefore had no /metrics endpoint, and no flag could give it one, because the repo's only MetricsServer instantiation is master_server.cpp's. Introduce MetricSink as the seam between measuring and publishing. MasterClient implements it unchanged, so with a master the metrics still go to the master -- the one behaviour this commit deliberately leaves alone. PrometheusMetricSink is the masterless counterpart: it owns a MetricsServer in this process and stamps the `node` label master used to add on ingest, so a dashboard query written for a master-backed cluster works against a standalone server, and two scraped servers do not collapse into one series. PoolClient picks its sink once in Init, and no emission site asks about masters again. Without a master it also starts its own tick thread, because the component counters are published by a timer that only MasterClient previously owned; Shutdown joins it before tearing down the backends it samples, with a final tick so a short-lived server still publishes. The endpoint is created only when there is no master. Its port comes from UMBP_STANDALONE_METRICS_PORT and defaults to 9092 rather than the master's 9091, so a node running both does not have two processes contending for one port. 0/off/false disables it; garbage is rejected loudly instead of being read as "off", because a typo discovered as a blank dashboard days later is the failure this is meant to prevent. A bind failure logs ERROR and continues: a server that stores KV but cannot be scraped beats one that will not start. The sink is injected after WithEmbeddedDefaults, not before. For a server started with no distributed environment -- the masterless default, and the case that needs a local endpoint most -- that function synthesizes `distributed` from a default-constructed struct and would have silently dropped the pointer. Tests cover the sink's counter/gauge/histogram forwarding, the node-label rules (stamp, do not overwrite, empty means none), and the port-resolution table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dpoint reachable Verifying the masterless metrics endpoint end to end turned up one real defect and two missing pieces of deployment glue. ResolveMetricsPortFromEnv used std::stol, which parses a numeric prefix and reports success for what it read. UMBP_STANDALONE_METRICS_PORT=9O92 (letter O) therefore resolved to port 9 -- privileged, so the bind fails, so the operator gets no metrics and an ERROR line about a port they never typed. That is exactly the "typo discovered as a blank dashboard days later" the resolver was written to prevent, so it now requires the whole string to be consumed. This is what the GarbageIsRejectedNotSilentlyDisabled test was already asserting and the only one of the nine that failed. Nothing told Prometheus the endpoint exists. examples/monitoring shipped one scrape fragment, for the master's 9091, so a masterless node published on 9092 and was never scraped -- the metrics reached a port and stopped there. Add the standalone fragment beside it, and say in the README which deployment wants which, with the point that the dashboards do not change: they are written against the series, and the server stamps the same `node` label the master used to add on ingest. UMBP_STANDALONE_METRICS_PORT was also absent from runtime-env-vars.md, where every other UMBP knob is documented. Verified on smci355-n09-29: a masterless standalone server (DRAM tier, no UMBP_MASTER_ADDRESS) serves /metrics on 9092, and after 64 x 1 MiB Put + Get through it reports backend bytes 67108864 committed and 67108864 resolved, every series carrying node="n09-29-test". 9/9 unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…INFO line
Answering "is this node evicting?" meant turning on DEBUG, which prints a line
per key: enough volume under steady offload to change the timing of the thing
being measured, and it drags every other DEBUG line in the process along with
it. The numbers were all being measured already -- they just had nowhere to go
in a run with no Prometheus attached, which is every run that is hard to debug:
a ctest, a one-shot benchmark, an sglang rank someone is bisecting, a customer
node.
ActivitySummary renders one window of that. It owns no thread, reads no clock
and calls no logger: it takes a cumulative snapshot and returns text, so the
deltas, the units and the idle case are unit-tested without standing up a pool.
PoolClient drives it on its own timer, in EVERY deployment -- master, masterless
or embedded -- because it is precisely the no-destination case that needs it.
UMBP_SUMMARY_INTERVAL_MS (default 60000, 0 disables); Shutdown joins it before
tearing down the backends it samples, with a final render so a run shorter than
one window still says what it did.
The vocabulary is the deployment's, not the slot lifecycle's: offload = commit,
load = resolve, evict = evict, with promote/demote the intra-pool transitions.
They are read through SampleMetrics(), not new accessors, so a new medium lands
in the summary with no code of its own; MetricPublisher is deliberately not
shared, since its delta baselines belong to the Prometheus path and one consumer
would steal the other's window.
Local eviction turned out to be invisible to metrics entirely. A page-backed
medium reclaiming to its low watermark frees pages inline instead of calling its
own Evict(), so no interface call is ever made and op="evict" stayed at zero
however much a masterless node reclaimed -- the DEBUG line was the only witness
there has ever been. PageBackend now publishes it as a medium event, which also
means it reaches Grafana for the first time.
Removed, because the summary now says the same thing better: the per-round
eviction line, and the eight per-key lines on the re-cache path. Their give-up
reasons became counters instead, which is the part DEBUG was worst at -- every
one of those paths is a best-effort return that leaves the read it was helping
perfectly correct, so a dropped re-cache was invisible unless you were already
printing a line per key. Each reason has a different fix (a bigger queue, a
wider admission policy, a larger tier), so they stay distinguishable.
An idle window still logs, carrying resident bytes and key count: a node holding
40 000 keys that moved none of them for a minute is the stall this is for, and
is indistinguishable from a healthy quiet node unless something says how much is
sitting there.
Verified on smci355-n09-29, 400 x 1 MiB into a 256 MiB tier:
8.0s: offload 400.00 MiB/400 keys (50.0 MiB/s) · load 192.00 MiB/192 keys
(24.0 MiB/s) miss=208 · evict 208.00 MiB/208 keys · promote 0 B · demote 0 B
DRAM resident 192.00 MiB/256.00 MiB (75%) 192 keys
400 offloaded - 208 evicted = 192 resident = 192 loaded, and the 208 evicted are
exactly the 208 misses. 13/13 new unit tests pass.
ctest against the parent commit in the same container: 11 failures there, 10
here, the difference being test_prometheus_metric_sink, which the previous
commit fixed. No new failures. The 10 are environmental -- they die in SetUp on
"rsmi_num_monitor_devices reported 0 GPUs", this node's 8 GPUs being fully
committed to another job.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # src/umbp/distributed/peer/backend/page_backend.cpp
…ut a master
Two gaps found verifying the masterless metrics endpoint against a live
Prometheus + Grafana.
Every counter series was born holding its first event. Sources skipped
zero-valued counters and MetricPublisher only forwarded a strictly positive
delta, so a series first appeared already carrying its first burst.
Prometheus computes rate() and increase() between two samples of one series,
so that burst was invisible: after 436 keys were locally evicted,
increase(mori_umbp_backend_medium_events_total{event="local_evict"}) read 0
and the eviction panels were empty; increase() of committed keys read 1039
against 1280 actually committed. A steady counter loses one tick, but a rare
one -- an eviction round, a no_space, a failed or rejected transfer -- loses
the whole event, and those are the ones dashboards and alerts exist for.
- MetricPublisher ships a counter the first time it sees it, even at zero.
- InstrumentedBackend publishes the (op, status) pairs each op can produce,
plus batches/seconds/bytes, from the first tick; impossible pairs stay out.
- PageBackend publishes its local_evict counters from the first tick when
local eviction is enabled.
- CompositeTransferEngine publishes every direction of each registered engine
and the rejected-items counter at zero.
- PoolClient seeds the per-traffic byte counters and ranged install failures
at zero, since those are written on the event itself.
A masterless node exported no capacity, live-key or client-count series:
the master derives them from heartbeats, so without one they did not exist
and the capacity / utilization / live-keys panels were blank. PoolClient now
publishes them itself when there is no master, under the master's names and
labels, so the same panels read either deployment.
Tests: a first-sight zero is shipped once and not re-shipped; a fresh
backend exposes its produced pairs at zero but not impossible ones; an unused
engine reports zero-valued series. Two tests that asserted the old
"no series until first event" shape are updated to expect zero.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
End-to-end validation of the eviction PR chain: passed. Branch
The log signatures of the 7 bugs this e2e originally found all stay at 0: whole-batch staging failures, Notes:
🤖 Generated with Claude Code |
Conflict in page_backend: keep main's side. #741 deleted PageBackend's own watermark loop (eviction now goes through PeerPool::Evict, so the backend's op="evict" already counts it), so this branch's PageBackend::SampleMetrics, its four local_evict_* counters, kPageBackendLocalEvictEvent and the activity summary's special case for them are dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What this does
A UMBP peer measured everything and, without a master, published none of it. This branch makes a masterless node report what it is doing — to Prometheus, and to its own log.
Three commits, each usable on its own:
1.
2c5d3871— serve metrics from a masterless nodeMasterClientwas the only metric destination in the process:PoolClient::CountMetricreturned early when it was null, and the component tick was registered only throughMasterClient::AddMetricsProvider. The standalone server — the deployment sglang's direct external linker actually runs — therefore had no/metricsendpoint, and no flag could give it one.MetricSinkbecomes the seam between measuring and publishing.MasterClientimplements it unchanged, so with a master nothing about the behaviour changes.PrometheusMetricSinkis the masterless counterpart: it owns aMetricsServerin-process and stamps thenodelabel master used to add on ingest, so a dashboard query written for a master-backed cluster works against a standalone server.Port comes from
UMBP_STANDALONE_METRICS_PORT, default 9092 rather than the master's 9091, so a node running both does not have two processes contending for one port.2.
b9973589— reject a mistyped metrics port, and make the endpoint reachableVerifying the above end to end turned up one defect and two missing pieces of deployment glue.
ResolveMetricsPortFromEnvusedstd::stol, which parses a numeric prefix and reports success for what it read.UMBP_STANDALONE_METRICS_PORT=9O92(letter O) resolved to port 9 — privileged, so the bind fails, so the operator gets no metrics and an ERROR about a port they never typed. That is exactly the "typo discovered as a blank dashboard days later" the resolver was written to prevent. It now requires the whole string to be consumed. This is what the existingGarbageIsRejectedNotSilentlyDisabledtest was already asserting, and the only one of its nine that failed.examples/monitoringshipped one scrape fragment, for the master's 9091, so a masterless node published on 9092 and was never scraped. Addsumbp_standalone.scrape.ymlbeside it, and a README table for which deployment wants which. The dashboards do not change — they are written against the series.UMBP_STANDALONE_METRICS_PORTwas absent fromruntime-env-vars.md.3.
35a5fe18— one periodic INFO line for offload / load / evict / promote / demoteAnswering "is this node evicting?" meant turning on DEBUG, which prints a line per key: enough volume under steady offload to change the timing of the thing being measured, and it drags every other DEBUG line along with it. The numbers were all being measured already — they just had nowhere to go in a run with no Prometheus attached, which is every run that is hard to debug: a ctest, a one-shot benchmark, an sglang rank someone is bisecting, a customer node.
ActivitySummaryowns no thread, reads no clock and calls no logger: it takes a cumulative snapshot and returns text, which is what makes the deltas, the units and the idle case testable without standing up a pool.PoolClientdrives it on its own timer, in every deployment — master, masterless or embedded — because it is precisely the no-destination case that needs it.UMBP_SUMMARY_INTERVAL_MS, default 60000,0disables.An idle window still logs, carrying resident bytes and key count: a node holding 40 000 keys that moved none of them for a minute is the stall this is for, and is indistinguishable from a healthy quiet node unless something says how much is sitting there.
Local eviction was invisible to metrics entirely
Worth calling out separately, because it is a pre-existing hole this branch closes rather than a consequence of the new log.
A page-backed medium reclaiming to its low watermark frees pages inline instead of calling its own
Evict(). No interface call is ever made, soInstrumentedBackendnever sees it andop="evict"stayed at zero however much a masterless node reclaimed. The per-round DEBUG line was the only witness there has ever been — and this branch removes that line.So
PageBackendnow publishes it as a medium event. Measured on a 256 MiB tier taking 400 × 1 MiB:Every byte of the
evict 208.00 MiBin the summary line above comes from that new event. This also means local eviction reaches Grafana for the first time — it was missing from both the log and the dashboards before.SSD is the other way round: its local eviction does go through the interface, so it lands in
op="evict"normally.CollectActivitySnapshotadds the two together; they are disjoint, so there is no double-counting, and both medium types report eviction in the same field.DEBUG logs removed
Nine sites, replaced by counters the summary aggregates — eight on the re-cache path in
pool_client.cpp(admission rejected, staging alloc failed, queue full ×2, fragmented slot, prefetched, re-cached, install failed) and the per-round eviction line inpage_backend.cpp.The give-up reasons became counters rather than being dropped, which is the part DEBUG was worst at: every one of those paths is a best-effort
returnthat leaves the read it was helping perfectly correct, so a dropped re-cache was invisible unless you were already printing a line per key. Each reason has a different fix — a bigger queue, a wider admission policy, a larger tier — so they stay distinguishable:Left alone deliberately: master-side eviction, routing decisions and slot-lifecycle housekeeping are not the five actions this summarises.
SsdCopyPipeline's three per-key drop logs look in scope, but that class is referenced nowhere outside its own file and unit test — dead code from a removed wiring — so they cannot fire in production. Its counters are labelled "Prometheus-only observability" and nothing publishes them; that is a separate cleanup, not this one.Design note
The summary does not go through Prometheus. Both are parallel readers of the same source — the components' cumulative counters, via the one
SampleMetrics()seam:MetricPublisher's baseline is deliberately not shared with the summary. Both need deltas from cumulative counters, so each subtracts the last value it saw; a shared baseline would let whichever ticked first consume the delta and leave the other reading zero. They also run at different rates (1 s vs 60 s).The upshot is that a new medium lands in both the summary and Grafana with no code in either reader — which is how the
local_evictfix above reached both at once.Testing
test_activity_summary(13) andtest_prometheus_metric_sink(9)./metricslive on 9092; after 64 × 1 MiB Put+Get it reports 67108864 bytes committed and 67108864 resolved, every series carryingnode=.UMBP_SUMMARY_INTERVAL_MS=0,UMBP_STANDALONE_METRICS_PORT=0).ctest, branch vs. parent commit, same container, same cmake configuration:
2c5d3871)The single difference is
test_prometheus_metric_sink, which commit 2 fixes. No new failures.The remaining 10 are environmental, not regressions. Every one of their 87 failed assertions had a single cause: adding
/dev/infinibandplus the ionic NIC mounts removed the RDMA errors entirely, and what is left dies inSetUponrsmi_num_monitor_devices reported 0 GPUs. All 8 GPUs on that node were fully committed to another job, so they could not be run there and remain unverified on this branch.🤖 Generated with Claude Code