Conversation
First component of the native multi-process Prometheus aggregation (prometheus_mp) plugin. mp_store is a per-process, fixed-slot memory-mapped store holding current metric state (cumulative counters and last-operation gauges) indexed by nixl_telemetry_event_type_t, for scrape-time aggregation by a single exporter process. It is a raw mmap of a fixed POD layout (no serialization): writers update slots in place with lock-free __atomic ops; a reader validates the magic/schema/size header and returns a consistent snapshot. Per-process identity (pid + /proc start_time, agent name, hostname, optional dp_rank) lives in the header for liveness and labeling. Built as a small prometheus-cpp-free static library with CTest coverage. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Second component of the prometheus_mp plugin. nixlMultiprocessCollector is a prometheus::Collectable that, on each scrape, globs the shared telemetry directory for peer store files, reads a snapshot of each, drops (and optionally reaps) stale ones, and returns per-process metric families. Series are emitted per (metric, process) with no cross-process aggregation: cumulative counters and last-operation gauges keyed by nixl_telemetry_event_type_t, plus agent_errors_total with a status label, labeled by hostname, agent_name and (when present) dp_rank. Staleness combines pid liveness (kill(pid,0) + /proc start_time to guard PID reuse) with a last-update TTL. The family-building and liveness logic are split into pure functions for unit testing; a shared store file-naming helper is added to mp_store. Collector library and tests are gated on prometheus-cpp. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
The collector's per-process series were disambiguated only by agent_name (plus optional dp_rank). If two processes share an agent name and have no dp_rank, they would emit an identical label set -> a duplicate Prometheus series and a rejected scrape. Add an unconditional pid label so every live process is a distinct series regardless of how callers name agents. pid (not the reserved "instance" target label) also avoids the exported_instance renaming trap. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Third component of the prometheus_mp plugin: the exporter that ties the store and collector together, selectable via NIXL_TELEMETRY_EXPORTER=prometheus_mp. Every process writes its own metric state to a per-process store in NIXL_TELEMETRY_MULTIPROC_DIR (unique file per pid/instance). On construction each process races to bind the scrape port: the winner runs a prometheus-cpp Exposer plus a nixlMultiprocessCollector that aggregates all peers on each scrape; losers fall back to writer-only mode. A bind collision is caught internally and never rethrown as nixlTelemetryBindFailed, so every process -- owner or writer -- gets a valid telemetry sink and all ranks are exported behind the single owner endpoint. Config: NIXL_TELEMETRY_MULTIPROC_DIR (required), NIXL_TELEMETRY_RANK_ENV (optional dp_rank source, default LOCAL_RANK), NIXL_TELEMETRY_MP_STALE_TTL; reuses NIXL_TELEMETRY_PROMETHEUS_PORT / _LOCAL. The exporter is built both as the installable plugin .so and as a static library for direct unit testing (owner mode, writer-mode-on-collision, missing-dir). Gated on prometheus-cpp. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Fourth component of the prometheus_mp plugin: an end-to-end test that proves cross-process aggregation with real processes. The parent becomes the bind-race owner and serves the endpoint; N children fork (while the parent is still single-threaded) and run as writers against the shared dir. After a single HTTP scrape of the owner port, all N+1 processes appear as distinct per-process series. One child is then killed and reaped, and a second scrape confirms its series is dropped and its store file removed (stale TTL 0). Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Handle processes that appear or die mid-scrape more robustly: - Reader treats a zero-magic store (a process still initializing its file, or an orphan from a process killed mid-creation) as a quiet skip instead of a "bad magic" WARN; a genuinely wrong non-zero magic still warns. - Collector now reaps unparseable store files (zero/bad magic, wrong schema, truncated) once they are older than max(stale TTL, 2s). The floor protects a store a live process is actively creating from being deleted out from under it, even when the TTL is 0. This closes a slow file leak: a process killed during store creation left a zero-magic file that was correctly skipped every scrape but never cleaned up. Cleanup is performed by the bind-race owner during Collect(), since a killed process cannot clean up after itself. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Describe the native single-endpoint multi-process aggregation exporter in docs/telemetry.md: add it to the architecture components, update the multi-process scrape note to point ranks needing full aggregation at it, and add a "Multi-process aggregation" section covering its model, configuration (NIXL_TELEMETRY_MULTIPROC_DIR/RANK_ENV/MP_STALE_TTL), labels, and the explicit limitation that it is hardcoded to NIXL's fixed metric model and cannot carry dynamic per-observation labels. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Document the multi-process Prometheus exporter plug-in alongside the sibling prometheus and doca plugin READMEs: how bind-race owner election and the per-process mmap stores work, configuration (NIXL_TELEMETRY_MULTIPROC_DIR and the optional RANK_ENV / MP_STALE_TTL / port vars), the per-process labels (hostname, agent_name, pid, optional dp_rank), and the explicit scope note that it is purpose-built for NIXL's fixed metric model (no dynamic per-observation labels) rather than a generic Prometheus multiprocess store. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Clarify that NIXL_TELEMETRY_MULTIPROC_DIR follows Dynamo's PROMETHEUS_MULTIPROC_DIR convention: a shared local folder (not NFS), one per pod/process-family, treated as ephemeral. Note the key difference from Dynamo: NIXL is loaded independently per rank with no parent to propagate the path, so the launcher/operator must set the same directory for every rank (hence it is required, not auto-defaulted). tmpfs is optional, not required. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
👋 Hi e-eygin! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the experimental ChangesPrometheus MP exporter
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Process
participant nixlTelemetryPrometheusMpExporter
participant ownerElection
participant storeWriter
participant PrometheusExposer
participant nixlMultiprocessCollector
Process->>nixlTelemetryPrometheusMpExporter: construct exporter
nixlTelemetryPrometheusMpExporter->>storeWriter: create per-process mmap store
nixlTelemetryPrometheusMpExporter->>ownerElection: attempt shared-directory election
ownerElection-->>nixlTelemetryPrometheusMpExporter: owner or writer-only result
nixlTelemetryPrometheusMpExporter->>PrometheusExposer: bind endpoint when elected
Process->>nixlTelemetryPrometheusMpExporter: exportEvent
nixlTelemetryPrometheusMpExporter->>storeWriter: write metric update
PrometheusExposer->>nixlMultiprocessCollector: scrape /metrics
nixlMultiprocessCollector->>storeWriter: read live snapshots
nixlMultiprocessCollector-->>PrometheusExposer: return MetricFamily list
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
CI pre-commit (codespell) flagged the local nowNs() helper (read as "knowns/nouns") and "unparseable". - Replace the duplicated per-file nowNs() helpers with the existing nixlTime::getNs() (CLOCK_MONOTONIC, host-wide so comparable across processes; also skew-free vs wall clock for the staleness delta). The last-update timestamp is only ever compared against another getNs() reading, so switching from system_clock to steady_clock is safe; the orphan-file mtime path is independent and still uses wall-clock time(). - Spell "unparseable" as "unparsable" in comments. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
Status, support, documentation and community
|
|
@coderabbitai resume |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/plugins/telemetry/prometheus_mp/mp_collector.cpp`:
- Around line 138-191: The reads in buildMetricFamilies are indexing fixed-size
snapshot arrays directly from telemetry_metric_event_types and
telemetry_error_event_types, so add bounds protection before accessing
snap.counters or snap.gauges. Update the buildMetricFamilies logic to use a
checked access path (such as .at() or an explicit range guard) for the slot
derived from the enum, and make sure the error-family loop uses the same
protection. Keep the existing telemetryMetricDescriptor, counterMetric, and
gaugeMetric flow unchanged otherwise.
- Around line 200-235: The Collect() scan still relies on the range-based for
over std::filesystem::directory_iterator, so an implicit iterator increment can
throw even though the constructor uses std::error_code. Update
nixlMultiprocessCollector::Collect() to handle iterator advancement errors
explicitly, either by switching to a manual directory_iterator loop that uses
the non-throwing increment overload with an error_code or by wrapping the
iteration in a try/catch that safely returns an empty result on
filesystem_error. Keep the existing dir_ scan, readStoreSnapshot, and reapStale_
logic intact while making the traversal resilient to mid-scan directory changes.
In `@src/plugins/telemetry/prometheus_mp/mp_store.cpp`:
- Around line 22-32: mp_store.cpp is using std::min and std::istream_iterator
without directly including their standard headers. Update the file’s include
list near the top to add both <algorithm> and <iterator> so MpStore’s
implementation does not rely on transitive includes.
In `@src/plugins/telemetry/prometheus_mp/mp_store.h`:
- Around line 41-47: The slot-count derivation in MP_STORE_SLOT_COUNT is fragile
because it assumes AGENT_TELEMETRY_EVENTS_DROPPED remains the last enum value.
Add a compile-time safeguard in mp_store.h, near the MP_STORE_SLOT_COUNT
definition, using a static_assert that validates the telemetry event enums fit
within the array bounds. Make the check cover the relevant
nixl_telemetry_event_type_t values used by mp_collector.cpp’s
buildMetricFamilies so future enum extensions fail at compile time instead of
causing out-of-bounds access.
In `@src/plugins/telemetry/prometheus_mp/prometheus_mp_exporter.cpp`:
- Around line 40-49: The telemetry constant declarations in
prometheus_mp_exporter.cpp use constexpr char[] while the matching constants in
mp_store.h use inline constexpr std::string_view, so make them consistent.
Update the identifiers such as defaultRankEnvName, prometheusPortVar,
prometheusLocalVar, multiprocDirVar, rankEnvVar, and staleTtlVar to use the same
string_view style as the sibling component, keeping the existing names and
usages intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 249ccc98-927b-47c3-95a4-ea65420aac52
📒 Files selected for processing (16)
docs/telemetry.mdsrc/plugins/telemetry/meson.buildsrc/plugins/telemetry/prometheus_mp/README.mdsrc/plugins/telemetry/prometheus_mp/meson.buildsrc/plugins/telemetry/prometheus_mp/mp_collector.cppsrc/plugins/telemetry/prometheus_mp/mp_collector.hsrc/plugins/telemetry/prometheus_mp/mp_store.cppsrc/plugins/telemetry/prometheus_mp/mp_store.hsrc/plugins/telemetry/prometheus_mp/prometheus_mp_exporter.cppsrc/plugins/telemetry/prometheus_mp/prometheus_mp_exporter.hsrc/plugins/telemetry/prometheus_mp/prometheus_mp_plugin.cpptest/gtest/meson.buildtest/gtest/telemetry_mp_collector_test.cpptest/gtest/telemetry_mp_e2e_test.cpptest/gtest/telemetry_mp_exporter_test.cpptest/gtest/telemetry_mp_store_test.cpp
✅ Action performedReviews resumed. |
… includes) - mp_store.h: add a compile-time static_assert that MP_STORE_SLOT_COUNT covers every telemetry event type the collector indexes, so extending the enum past AGENT_TELEMETRY_EVENTS_DROPPED fails the build instead of causing an out-of-bounds counter/gauge access. - mp_collector.cpp: iterate the shared directory with the non-throwing increment(ec) instead of the range-for's throwing operator++, since peer writers and this collector's own reaping mutate the directory concurrently and a mid-iteration filesystem error would otherwise escape Collect(). - mp_store.cpp: add explicit <algorithm> and <iterator> includes (std::min, std::istream[buf]_iterator) rather than relying on transitive includes. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
/build |
|
/ok to test 3bf2139 |
The optional per-process rank label was named dp_rank, but its value is sourced from the local/per-GPU (tensor-parallel) rank env (default LOCAL_RANK), not the data-parallel rank. That misdescribed the value and collided with Dynamo's own data-parallel dp_rank series. Rename the emitted label and the internal identifiers (store field, writer param, exporter helper) to local_rank. The NIXL_TELEMETRY_RANK_ENV var and its LOCAL_RANK default are unchanged; the label is still optional and emitted only when the env is set. Docs and tests updated. Part of NIX-1614. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
The prometheus_mp store writer only unmapped its file on destruction, so an agent destroyed while its process keeps running left a store behind. Because liveness is pid-based, the collector kept publishing that dead agent's frozen series indefinitely. Have ~mpStoreWriter() best-effort remove its own file: clean shutdown is the deterministic "producer gone" signal, while crash/kill (destructor never runs) still falls back to the owner's liveness/TTL reaping. Adjust the store round-trip tests to read while the writer is alive (the real cross-process pattern) and add a test for the new cleanup. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
Multiple agents created in one process with the same agent name shared every series label (hostname, agent_name, pid, local_rank), producing duplicate Prometheus series that make the whole scrape fail. The store filename already carried a per-process instance counter, but it never reached the exposed labels. Persist the instance in the store header and emit it as an agent_instance label, so same-name same-process agents get distinct series (agent_instance=0 for the common single-agent case). Schema version is unchanged: prometheus_mp has not shipped, so no on-disk format compatibility is at stake. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
🤖 CI Triage Agent — TL;DR: The tsan variant of the "Test Sanitizer" stage failed because the earlier Full analysisSummary: Root cause: The sanitizer runner accumulates failures into a variable, and at the end of the tsan run that variable held Implicated commit: unknown — the specific racing component is in the TSan report inside the archived File: Failure marker at end of the tsan "Test Sanitizer" stage log; detailed race report in artifact Suggested fix:
Related: PR #1920 (under test); possibly related threading/test-isolation work: PR #1743 ("Run in single process"), PR #1610 ("PLUGINS/UCX: Do not allow emulated RMA protocols"). CI sanitizer harness introduced in #1709. |
|
🤖 CI Triage Agent — TL;DR: The "Run CPP tests" stage (#242) was killed with exit code 143 (SIGTERM) after ~61 minutes because the Full analysisSummary: Root cause: The gtest suite is parametrized over multiple UCX configurations, and the Implicated commit: No single functional regression commit; the slow variants trace to the threadpool test parametrization introduced/derived in File: Suggested fix: Reduce the wall-clock cost of the Related: PR #1920 (trigger, telemetry — not the cause); #1906 (threadpool engine refactor). none other directly matching. |
|
🤖 CI Triage Agent — TL;DR: The build compiled fine; the job failed in "Allocate DL EP Environment" because a Slurm Full analysisSummary: Stage 200 "Allocate DL EP Environment" failed with Root cause: Slurm resource allocation timeout. At Implicated commit: none — this is a CI infrastructure/cluster-capacity failure, unrelated to commit 548b07b or PR #1920. File: Jenkins pipeline Slurm allocation step ( Suggested fix: Re-run the build; the Related: none found. |
The repo builds with cpp_std=c++20, so the prefix/suffix checks on store and lock file names no longer need positional compare() calls. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
A store is now flock-ed before it has a name -- created nameless with O_TMPFILE and linked in once initialized, or staged under a .staging name and renamed where the filesystem has no O_TMPFILE -- and stays locked for its writer's lifetime. A store whose lock can be taken therefore has no writer left and will never change again, which is the whole liveness test. That replaces kill(pid, 0), the /proc start-time reuse guard and the heuristics that protected a store mid-creation (the zero-magic grace period and its two-second floor), and drops the requirement that ranks share a PID namespace with the collector. The stale TTL still decides how long a departed writer's final values are published before its file is reaped. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
The warning promised that such a store would "linger for the stale TTL after it exits", which is not what happens in either direction. A reader that can lock the store reads it as abandoned, so the rank loses its series and its file if it goes quiet for longer than the TTL while still running; where no process can lock at all, the reader's probe fails too and nothing is reaped, so departed ranks stay published indefinitely. Say both, and give the README's "never dropped for being idle" the same lock-is-usable qualifier the election already carries. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
/build |
|
/ok to test 572458f |
ColinNV
left a comment
ColinNV
left a comment
There was a problem hiding this comment.
I can't find any more race conditions.
An mmap owner in src/utils/common replaces the writer's raw pointer plus size pair and the reader's unique_ptr-with-deleter guard, which removes the unmap-and-rethrow path from the writer's constructor and empties its destructor. The store file name needs nothing more from /proc than a value two runs sharing a pid cannot both have, so a per-process constant taken from the wall clock replaces parsing field 22 of /proc/<pid>/stat. Also states why an unprobable store reads as held. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
|
/build |
|
/ok to test 8f25980 |
## What? Three gtest cases for the `prometheus_mp` telemetry-directory hardening in `resolveMultiprocDir()`: a directory the exporter creates is 0700, and an existing group-writable or world-writable one warns and still serves. Tests only, no production change. ## Why? Deferred from the diff freeze on #1920 (NIX-1683). None of it could be observed before: the mp fixture hands the exporter a directory it already created at 0700, so the warning can never fire and the production `chmod` is indistinguishable from the fixture's own. Dropping the `created` guard, or reordering `permissions` before `create_directories`, broke nothing visible. The foreign-owner branch (`st_uid != geteuid()`) is left uncovered by design, not by oversight — see below. ## How? All three tests point `NIXL_TELEMETRY_MULTIPROC_DIR` at a **subdirectory** of the fixture's directory. That child path is free to be absent or loose without any fixture surgery, and the fixture still needs its own 0700 chmod to stop a permissive CI umask from reddening the suite. <details> <summary>Mutation testing, the two loose cases, and the uncovered branch</summary> Each test was verified against the mutation it exists to catch, and confirmed to fail on that one alone: | Mutation | Result | |---|---| | Drop the `permissions(..., owner_all, replace)` call | `CreatedTelemetryDirIsPrivate` fails: mode is the umask default 0755 | | Drop the `created` guard (chmod unconditionally) | `WorldWritable...`/`GroupWritable...` fail: the loose directory is silently fixed before `stat()`, so the warning count falls to 0 | | Narrow the mask to `S_IWGRP` alone | `WorldWritableTelemetryDirWarns` fails; the group case still warns | | Narrow the mask to `S_IWOTH` alone | `GroupWritableTelemetryDirWarns` fails; the world case still warns | The last two are why the loose case is 0720 and 0702 rather than a single 0777: 0777 sets both bits, so it cannot tell the halves of the mask apart. Either single-bit case subsumes 0777. They are two `TEST_F`s rather than a `TEST_P` because the fixture derives its directory from the suite and test name, and an instantiated parameterised suite puts a `/` in both, nesting that directory and leaving the intermediate level behind `TearDown`. `CreatedTelemetryDirIsPrivate` also asserts the subdirectory is absent first: otherwise `create_directories()` reports `created == false`, the `chmod` never runs, and the 0700 assertion is satisfied by whatever left the directory behind — which a failed `TearDown` under `--gtest_repeat` could do, since every iteration reuses the same path. All mutations were reverted. Verified with `--gtest_repeat=5`: 93 tests per iteration, all passing, no un-ignored warnings. **Foreign owner.** Covering it needs `chown`, so the test would run only as root and skip on every unprivileged lane — an assertion whose meaning depends on the environment, which is the shape NIX-1710 just removed from the tracing suite, and which NIX-1623 is trying to make fatal. The prerequisite is a second unprivileged uid in the test container; that is CI infrastructure and belongs in its own change. </details> --------- Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
3 participants
What?
Adds
prometheus_mp, a native telemetry exporter plug-in that exposes the telemetry of all processes of a multi-process NIXL run (TP/DP/PP) behind a single Prometheus scrape endpoint — with no external dependency (no DOCA/DTS, not built on Python'sprometheus_client).Enable with
NIXL_TELEMETRY_EXPORTER=prometheus_mpand point every rank at one shared directory viaNIXL_TELEMETRY_MULTIPROC_DIR.Why?
The existing native Prometheus exporter binds one scrape port per process. Under multi-process parallelism every rank creates its own agent and tries to bind the same port; only one wins, so every other rank's telemetry is lost by design (reported for a Dynamo + SGLang TP=8 deployment, issue #1838). The existing exporter can only make that collision benign (WARN and continue) — it can't recover the lost ranks.
This makes every rank's telemetry available behind one endpoint, natively — no external aggregator (the users run the native exporter specifically to avoid extra infrastructure) and no parent process to coordinate (NIXL is loaded independently per rank, so ownership is self-elected at runtime). It complements, and does not replace, the DOCA/CollectX exporter (which aggregates via an external service).
Tracking: #1838 (#1838 / NIX-1614).
How?
Every process writes its own metric state to a fixed-slot, memory-mapped file in the shared dir. One of them is elected to serve: the processes race for an exclusive
flockon a lock file named after the address they were configured to serve, and only the winner (owner) binds the scrape port, running theExposerplus a customprometheus::Collectablethat, on each scrape, reads a snapshot of every live peer and republishes it. The losers run writer-only and never bind, so losing is benign and no rank is dropped. Each process is its own series (no cross-process summing, so counters stay monotonic).The owner is not a single point of failure: the kernel frees the lock when it dies, and a writer re-running the election takes the endpoint — and the reaping — over.
The exported metric set is at full parity with the single-process
prometheusexporter: counters, last-operation gauges, error counters, and the transfer-duration histograms.Design detail (election, crash-safety, histograms, labels, cleanup, config, limitations, tests)
Owner election & failover
The lock, not the bind, is what elects: two ranks binding concurrently cannot tell which of them got there first, so gating the bind on an exclusive lock is what makes exactly one process serve. That guarantee holds as long as the lock is usable — if the lock file cannot be opened, is not a regular file owned by the run's user, or sits on a filesystem without
flock, every process warns and falls back to the port bind deciding.Naming the lock file after the address keeps it contentless and scopes the election to the ranks that would actually collide, which turns two otherwise silent misconfigurations into warnings. An owner that cannot bind reports the port as held from outside the run (a foreign service, or a rank pointed at a different directory) and concedes the election rather than holding it, so the next rank to win takes the address over once the port frees. A directory served on more than one address means the ranks disagree on the port; each serves what it was configured with, but every one of them exports every rank, so a Prometheus scraping more than one target sees the same series twice. Owners find each other by trying the directory's other lock files: one that can be locked is a leftover from an earlier run, one that cannot is a live second owner.
A process that is not serving re-runs the election as it exports, throttled to a few times a second, and binds if it now wins. The endpoint is therefore unreachable for that gap plus up to one scrape interval when the owner dies, rather than for the rest of the run. Two consequences: a rank that exports nothing never re-elects, so a run that goes fully idle at the wrong moment stays down until any rank produces telemetry again; and when the port is held from outside the run, the retry backs off to every few seconds instead of hammering a bind that cannot succeed. Alert on the scrape target's
upmetric rather than on absent series.Crash- & race-safety
The store is a raw mmap of a fixed POD layout, so it must survive concurrent readers, processes appearing mid-scrape, and processes killed mid-update:
__atomicop —SIGKILLcan't tear it; a process killed mid-batch just leaves the store a few increments behind (metrics are snapshots, not a ledger).magic, published last, still guards the staging path used on filesystems withoutO_TMPFILE.open/mmapfailure leaves a live peer's store untouched.Collect()cannot take down the process. prometheus-cpp calls it on its HTTP handler thread, so directory iteration is non-throwing and the body is wrapped in a catch-all that degrades to an empty scrape.static_asserton its size, so a reordered field or a changed cap fails the build instead of shifting offsets under a peer that still validates the header.Duration histograms
agent_xfer_time_us/agent_xfer_post_time_usare exported per process as the usual_bucket{le="..."}/_sum/_countseries, with the same names and default bounds as the single-process exporter. Keeping them in a fixed mmap layout has two consequences:sample_countfrom one read makes consistency structural rather than timing-dependent.NIXL_TELEMETRY_HISTOGRAM_BUCKETS_USfrom its own environment and the collector cannot know what a peer was configured with. Give every rank the same value, or the family ends up with series carrying differentlesets. A fixed layout also has to cap the list: an override longer than 32 bounds is rejected at construction rather than silently truncated — the only behavioural difference from the single-process exporter.Series & labels
Labeled
hostname,agent_name,pid(cross-process uniqueness; deliberately not the reservedinstance),agent_instance(distinguishes multiple same-name agents in one process;0in the common case), and optionallocal_rank(local/per-GPU rank, only when the rank env is set).agent_errors_totaladditionally carries the boundedstatuslabel. Names/types/semantics are identical to the single-processprometheusexporter.Cleanup & lifecycle
A departing process leaves its store behind, cleanly or not: its last values are usually unscraped, and unlinking on exit would drop everything recorded since the previous scrape. The owner reaps lazily during
Collect(), and liveness is a lock rather than a pid — each processflocks its store before the file has a name (created nameless withO_TMPFILE, linked into the directory once initialized) and holds it for its lifetime, so a store the owner can lock has no writer left and will never change again. Such a store is published until its last update ages past the TTL, then reaped: the same path for a clean exit and aSIGKILL, with no pids, no/proc, and no shared PID namespace. Whole-run cleanup is left to the deployment (e.g. a per-podemptyDir).Configuration
NIXL_TELEMETRY_EXPORTER=prometheus_mpNIXL_TELEMETRY_MULTIPROC_DIR/dev/shmto stay in RAM)NIXL_TELEMETRY_PROMETHEUS_PORT/_LOCALNIXL_TELEMETRY_RANK_ENVlocal_ranklabelLOCAL_RANKNIXL_TELEMETRY_MP_STALE_TTLNIXL_TELEMETRY_HISTOGRAM_BUCKETS_USMust be a local filesystem (mmap
MAP_SHAREDcoherence) — not NFS. Ranks must also agree on a host-wideCLOCK_MONOTONIC(a shared time namespace); there is no PID namespace requirement, since liveness is the store's own lock. A missing directory is created0700; an existing one is left alone, with a warning when it is group- or world-writable or owned by another user, and store files planted by another user are ignored rather than read. Unlike a bind collision, a configuration error is fatal: a missingNIXL_TELEMETRY_MULTIPROC_DIR, or a bucket override longer than 32 bounds, failsnixlAgentconstruction rather than degrading.Scope / limitations (intentional)
Purpose-built for NIXL's fixed, low-cardinality, per-process label model — it can't represent a metric with a dynamic/high-cardinality label that varies per observation (none exist today; that would need a keyed store). The
pid/agent_instancelabels that keep per-process counters monotonic also make a restarted rank a new series, so a crash-looping deployment grows TSDB cardinality at the restart rate (scrape size still follows the live process count).Testing
Runs under CTest/meson: 46 unit tests over the store, the collector and the exporter — including histogram bucket-edge semantics, cumulative/
+Infemission, rejection of an over-long bucket override, each election outcome, a writer promoting itself when the owner exits, the two invariants that keep failover safe (re-electing while already serving cannot drop the held lock, and a conceded election really does free the endpoint), and the lock-liveness contract (a writer holds its own store and releases it on destruction, a live writer outlives a TTL that expires everything, and a departed writer's final values are published once more before its file is reaped) — plus a forking multi-process e2e that scrapes the owner over HTTP and asserts all ranks aggregate, the histogram_bucket/_sum/_countseries are served, and a killed rank is dropped and reaped. Allprometheus_mptests pass; changed lines are clang-format-19 clean; every commit is DCO-signed.Summary by CodeRabbit
prometheus_mpmultiprocess Prometheus exporter that exposes a single/metricsendpoint and aggregates per-process telemetry.pid,agent_instance, optionallocal_rank).agent_errors_totalmetric family naming/help across Prometheus exporters.