[e-eygin]

What?

Removes a per-event std::string allocation in the DOCA telemetry exporter's counter
hot path. In nixlTelemetryDocaExporter::appendCounterSample, the event name is now
taken directly from nixlEnumStrings::telemetryEventTypeStr(event.eventType_).data()
instead of constructing a temporary std::string just to call .c_str().
telemetryEventTypeStr is constexpr and returns a std::string_view backed entirely
by null-terminated string literals (every case plus the "unknown_event" default), so
.data() is a valid C string for the DOCA Metrics API. The -Wdeprecated-declarations
pragma around the experimental DOCA call is kept as-is.

Why?

Pure hot-path overhead: a heap allocation on every exported counter event with no
behavioral benefit. This is the DOCA twin of the already-merged Prometheus Perf-5
(#1878 / NIX-1588), which removed the equivalent allocation on the Prometheus side.
Tracking: NIX-1591.
Behavior-preserving — identical series names, labels, and values. Covered by the existing
doca_nixl_test (test/doca-telemetry/telemetry_doca_nixl_test.cpp), which asserts counter
series names and cumulative values via the OpenMetrics scrape parser. All 5 tests pass.

How?

Scope is a single site: the gauge path already takes a const char * (gaugeMetricName)
and the error path uses the "agent_errors_total" literal, so appendCounterSample was the
only allocation site. Exporter-side only — independent of the queue-architecture decisions.

Summary by CodeRabbit

• Refactor
  • Improved telemetry export performance by reducing unnecessary string copying when reporting counter increments.
  • Streamlined how event/metric names are passed to the telemetry counter reporting call, aligning formatting with the updated data flow.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

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

🚀

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 21def64b-2013-4a71-ace1-99692acbbf49

📥 Commits

Reviewing files that changed from the base of the PR and between 6829b3d and 9ed5bfa.

📒 Files selected for processing (1)

• src/plugins/telemetry/doca/doca_exporter.cpp

---

📝 Walkthrough

Walkthrough

The change updates appendCounterSample in doca_exporter.cpp to pass the telemetry event name as a const char* from nixlEnumStrings::telemetryEventTypeStr(...).data(), removing the intermediate std::string and c_str() conversion.

Changes

DOCA Telemetry Exporter Update

Layer / File(s)	Summary
Counter sample event name derivation src/plugins/telemetry/doca/doca_exporter.cpp	event_name now comes directly from telemetryEventTypeStr(...).data() and is passed into doca_telemetry_exporter_metrics_add_counter_increment without a local std::string copy.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: iyastreb, ColinNV

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely describes the main change: removing a DOCA counter event name allocation.
Description check	✅ Passed	The description follows the What/Why/How template and provides the optimization rationale and implementation details.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[e-eygin]

/build

[e-eygin]

/ok to test 9ed5bfa

[svc-nixl]

👀 Investigating nixl-ci-gpu build 2519…

[svc-nixl]

🤖 CI Triage Agent — nixl-ci-gpu · commit 2f4c0c08

This confirms the mechanism. The two branches were running concurrently on the same SLURM node (mizu01), and both share /tmp. Note branch 2's get_next_tcp_port writes to /tmp/nixl_tcp_port_8 and repeatedly fails with "No such file or directory" — /tmp itself is gone. And branch 1 died with test_plugin: No such file or directory while its cwd was being wiped.

The trigger is that when the first branch failed (exit 127 in branch 1's test_plugin step, and the pipeline uses --halt now,fail=1 / retry with a cleanup stage), the pipeline's stopAllForBuild / workspace cleanup (rm -rf .../workspace/* seen in the "pipeline_stop" stage) tore down the shared working tree and /tmp while branch 2 was still executing tests on the same node. Branch 2 then saw getcwd() failed and Parent directory (/tmp/) does not exist.

The root of branch 1's original failure: ./bin/test_plugin: No such file or directory — the binary is invoked by relative path (test_cpp.sh line 19/./bin/test_plugin) but at that moment the cwd had already been removed, because the concurrent-branch cleanup raced in. Both symptoms are the same underlying issue: two parallel matrix branches sharing one node's filesystem//tmp, and one branch's teardown deletes the workspace//tmp out from under the other still-running branch.

This is not a hang (timestamps show continuous test progress up to the failure) and not a legitimate test-logic bug — it is a CI infrastructure race between concurrently-scheduled parallel branches on a shared node.

Summary: nixl-ci-gpu CPP test job failed with exit code 127 — the running job's working directory and /tmp were deleted out from under it mid-run.

Root cause: A filesystem race between the two parallel build/test branches (ucx-master/1 and ucx-v1.21.x/2) running on the same SLURM node mizu01 and sharing the same workspace and /tmp. When one branch failed/finished, the pipeline cleanup (workspace rm -rf in the "pipeline_stop"/stopAllForBuild stage) removed the working tree and /tmp while the other branch was still executing. The tell-tale errors are FileNotFoundError: ... '/tmp/gtest_parallel_x87wox6a.log', ./bin/test_plugin: No such file or directory (branch 1) and sh: 0: getcwd() failed: No such file or directory / Parent directory (/tmp/) does not exist (branch 2). Test execution was progressing continuously (per-test timestamps steady up to the failure), so this is not a hang or a per-test bug.

Implicated commit: unknown (CI infrastructure/parallel-scheduling issue, not a source regression; the two touched CI scripts most recently were be4bb2225399 by e-eygin and [REDACTED:Hex High Entropy String] by Guy Ealey Morag, but neither introduces the race)

File: .gitlab/test_cpp.sh:19 (./bin/test_plugin invoked via relative path) and .ci/scripts/common.sh:50,66 (get_next_tcp_port writing /tmp/nixl_tcp_port_8) — these fail because cwd//tmp were deleted, not because the scripts are wrong.

Suggested fix: Isolate the two parallel branches so one branch's teardown cannot delete the other's filesystem: give each matrix branch its own unique workspace directory and a per-branch TMPDIR (e.g. TMPDIR=$(mktemp -d) per branch) instead of sharing /tmp, and ensure the stopAllForBuild/workspace-cleanup stage only runs after all parallel branches complete (or scopes cleanup to per-branch job IDs/dirs). As a secondary hardening, have test_cpp.sh verify its cwd exists / run binaries by absolute path so a wiped cwd fails fast with a clear message. Do not raise any time limit — the job was making steady progress, not hanging.

Related: none found

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 3bd38a23-efe6-4a44-8a83-67ce742f15dc in the triage console for the audit trail.

[svc-nixl]

👀 Investigating nixl-ci-gpu build 2520…

[svc-nixl]

🤖 CI Triage Agent — nixl-ci-gpu · commit 2f4c0c08

I have enough evidence now. Let me summarize the timeline analysis conclusively.

Timeline analysis

The nixlbench benchmark loop ran through all combinations (UCX/UCCL × READ/WRITE × DRAM/VRAM) with continuous output up to:

• 11:56:49.858 — last benchmark result printed (UCCL WRITE DRAM→DRAM, 2.39 GB/s)
• 11:56:52.401 — Engine destroyed (final cleanup of the last worker pair)
• 11:56:53.339 — + kill -9 3000924 (the test harness kills the background ETCD/coordinator PID at end of the loop)
• … 37 minutes of total silence …
• 12:33:56.023 — Cancelling nested steps due to timeout → SIGTERM → script returned exit code 143

The largest inter-line gap is ~37 minutes (11:56:53 → 12:33:56), which is essentially the entire remaining wall-clock budget. Per triage rules, this is a hang, not slow work. The exit code 143 / "Cancelling due to timeout" is only the symptom.

The last meaningful action before the silence was kill -9 3000924. The benchmark work itself completed successfully (every configuration produced a result line, no errors, no failing assertions). What hung is the shutdown/teardown after the final kill -9 — the shell did not return to the pipeline after killing the background process. The functional test payload passed; the process simply never exited, so Jenkins killed it at the wall-clock limit.

Notably, PR #1886 ("telemetry: avoid DOCA counter event name allocation") has nothing to do with the UCX/UCCL benchmark loop or ETCD teardown that hung. The only in-run anomalies are benign warnings: repeated ibv_query_port_speed(...) failed: Protocol not supported (UCX, environmental — IB link/emulation quirk) and Failed to set thread affinity to NUMA node 1 (UCCL, environmental — cgroup/NUMA restriction in the k8s pod). Neither is fatal.

Summary: The nixlbench GPU test job was killed by the Jenkins wall-clock timeout (exit 143) after hanging for ~37 minutes; all benchmark work had already completed successfully.

Root cause: A hang during post-benchmark teardown. The last application activity was kill -9 3000924 (the background ETCD/coordinator process) at 11:56:53, immediately after the final Engine destroyed. Nothing printed for the next ~37 minutes until the timeout kill. The benchmark payload itself passed (every UCX/UCCL × READ/WRITE × DRAM/VRAM combination produced a valid result line, no errors). The process/shell failed to exit after the background PID was reaped — a teardown/wait hang, not slow computation. This is unrelated to PR #1886's content (telemetry DOCA counter change), which does not touch the UCX/UCCL benchmark loop or ETCD lifecycle.

Implicated commit: unknown (the hang is in the benchmark test harness teardown / ETCD-reaping path, not in the PR's changed files; the harness run_nixlbench_two_workers_etcd loop and its final kill -9 $PID are the relevant code, but the responsible commit cannot be pinned from the log alone)

File: benchmark test harness teardown path — the run_nixlbench_two_workers_etcd loop and its trailing kill -9 <etcd_pid> (the exact script was not retrievable; closest analogue is the ETCD lifecycle in .ci/scripts/common.sh:150 start_etcd_server and the CPP harness teardown at .gitlab/test_cpp.sh:126)

Suggested fix: Do not treat this as needing a longer timeout. Investigate why the shell does not return after kill -9 3000924: (1) confirm the background nixlbench/etcd children are all reaped — kill -9 on one PID may leave orphaned parallel/worker or GPU processes blocking a subsequent wait; (2) add an explicit wait with a timeout plus a process-group kill (kill -9 -- -$PID / pkill -P) in the teardown so a stuck child cannot hold the job open; (3) add a trap-based cleanup that force-terminates all spawned PIDs and logs before exiting, so the next hang is diagnosable. Re-run to confirm the hang reproduces independently of PR #1886 (it very likely does — this looks like a flaky/infrastructure teardown hang).

Related: PR #1886 (the triggering PR, #1886 — content unrelated to failure); prior ETCD-cleanup fix #1227 ("prevent pkill etcd from killing other containers' processes") is the most relevant precedent for the ETCD-teardown code path.

[e-eygin]

@ColinNV @ovidiusm please review

[ColinNV]

Calling data() on a string_view and using the result as C-string should not be done, even in this case where the views happen to be, for now (!), constructed from C-strings, it's just such an anti-pattern. Perhaps return C-strings from the enum-to-name function?

[e-eygin]

Superseded by merged #1894 (metric parity): the DOCA counter name now comes from the shared const char * telemetryMetricDescriptor, so the per-event std::string this PR removed no longer exists on main. Closing — thanks @ColinNV for the review!