Skip to content

fix(ingest): convert tachometer parquet that lacks timestamp_ns - #414

Merged
nv-yna merged 2 commits into
NVIDIA:mainfrom
nv-yna:yna/tachometer-ingest-time-since-start
Sep 11, 2026
Merged

nv-yna merged 2 commits into
NVIDIA:mainfrom
nv-yna:yna/tachometer-ingest-time-since-start

Conversation

@nv-yna

@nv-yna nv-yna commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

What

src/ingest/metrics_tachometer.py now converts tachometer parquet that has no timestamp_ns column. When the column is absent, the epoch of time_since_start == 0 is taken from, in order:

  1. an explicit --start-ns (new CLI flag; process(..., start_ns=)),
  2. the first timestamped line of the run's tachometer.out (searched up to four levels above the parquet; the scraper logs within the second it starts counting from),
  3. the newest parquet's mtime minus its largest time_since_start (row-group statistics, no data read).

The source used is logged at WARNING. Precision equals the one-second grid every timestamp is snapped to anyway. Parquet that carries the real column is processed exactly as before.

Why

Every published tachometer-scraper release asset up to v1.0.96 predates the vendored writer's timestamp_ns column (#350, merged 2026-08-26). The release workflow's build-tachometer-scraper jobs have failed at actions/checkout@v4 on every merge that touched the Rust sources since then (runs 33043846919 for #350 and 34112274235 for #401, both jobs failing within 4 s), the release job was skipped, and every later release took the "reuse scraper binaries from the previous release" path. Result: the v1.0.96 aarch64 asset is byte-identical (sha256 344181ae…) to a download from 2026-08-27, and make setup installs it.

Such scrapers write only time_since_start, and the ingest died on every capture:

File ".../src/ingest/metrics_tachometer.py", line 226, in process
    tss = batch.column("timestamp_ns").to_pylist()
KeyError: 'Field "timestamp_ns" does not exist in schema'

Seen on hecate job 487539 (2026-08-27) and again on 565810 (2026-09-09). The metrics leg of the perf dashboard bundle was therefore silently empty for runs that had the full capture on disk.

The release workflow itself needs a separate fix (the pull_request_target checkout of merge_commit_sha); until the assets are refreshed, make tachometer-scraper (or the Dockerfile cross-build) produces a correct binary. This change makes the ingest independent of which binary a run happened to use.

Why draft

Held until the three hecate runs that exercise it finish and post-process end to end: 565810 (old binary, fallback path) and 565811 / 565854 (rebuilt binary, timestamp_ns present). Will attach the ingest log lines from each before marking ready for review.

Validation

  • tests/test_metrics_tachometer.py: 4 new tests (log-anchored, mtime-anchored, explicit --start-ns, and real-column-preferred-over-stale-log) + 8 existing → 12 passed; pytest -k "tachometer or ingest or metrics" → 102 passed.
  • Real data: job 565810's live legacy parquet (11,007,786 rows, 17 min of capture) → 1,010 one-second timestamps 2026-09-10T00:37:10Z..00:54:02Z, anchored on the scraper's first log stamp 00:37:08Z; 125 families in the first line. ~2.5 min on the login node.
  • Lint: ruff check on the two files reports the same 5 findings main already has (RUF100, SIM108, SIM115, RUF046 on the pre-existing snap line); this diff adds none.

🤖 Generated with Claude Code

Every published tachometer-scraper release asset up to v1.0.96 predates the
vendored writer's timestamp_ns column (NVIDIA#350, 2026-08-26): the release workflow's
build-tachometer-scraper jobs have failed at actions/checkout since the NVIDIA#350
merge (runs 33043846919, 34112274235), and every release since re-uses the
previous release's binaries. Such scrapers write only time_since_start, and the
ingest died on every capture with

    KeyError: 'Field "timestamp_ns" does not exist in schema'

(seen on hecate 487539 and again on 565810), so the metrics leg of the perf
dashboard bundle was silently empty for runs that had the data on disk.

When the column is absent, derive it: epoch of time_since_start == 0 from an
explicit --start-ns, else the first timestamped line of the run's tachometer.out
(the scraper logs within the second it starts counting from), else the newest
parquet's mtime minus its largest time_since_start (row-group statistics). The
source is logged at WARNING; precision equals the one-second grid every
timestamp is snapped to. Parquet with the real column is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@b943e59). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #414   +/-   ##
=======================================
  Coverage        ?   75.05%           
=======================================
  Files           ?      109           
  Lines           ?    16002           
  Branches        ?        0           
=======================================
  Hits            ?    12011           
  Misses          ?     3991           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nv-yna

nv-yna commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Evidence from the three hecate runs (2026-09-09, AgentX baseline recipe, 8 VR200 nodes)

All three postprocessed with this branch's metrics_tachometer.py (cherry-picked onto the run checkouts). Lines are from outputs/<job>/logs/sweep_<job>.log.

565810 — release-asset scraper (v1.0.96 binary, no timestamp_ns column) → fallback path taken:

19:21:28 INFO [L2 metrics] auto-selected source: tachometer
19:21:28 INFO [L1] metrics raw: .../565810/logs/tachometer/raw/scrape/incomplete-49.parquet (tachometer parquet)
19:21:29 WARNING timestamp_ns absent from the tachometer parquet (scraper predates #350); deriving timestamps from time_since_start + 2026-09-10T00:37:08+00:00 (.../565810/logs/tachometer.out)
19:30:31 INFO [L2 metrics] tachometer -> server_metrics_export.jsonl: 6106 timestamps, dedup 6106 -> 6106 lines
19:32:15 INFO [done] bundle ready in 649.3s: aiperf=True traces=False metrics=True request_trace=False

Without this change the same step raised KeyError: 'timestamp_ns' and the bundle had no metrics (the failure that started this PR). Parquet inventory for that run: incomplete-49.parquet | rows: 70048941 | has timestamp_ns: False.

565811 and 565854 — scraper rebuilt from source (has timestamp_ns) → unchanged fast path, no warning:

21:19:44 INFO [L1] metrics raw: .../565811/logs/tachometer/raw/scrape/incomplete-21.parquet (tachometer parquet)
21:22:53 INFO [L2 metrics] tachometer -> server_metrics_export.jsonl: 5661 timestamps, dedup 5661 -> 5661 lines
22:02:19 INFO [L1] metrics raw: .../565854/logs/tachometer/raw/scrape/incomplete-50.parquet (tachometer parquet)
22:10:05 INFO [L2 metrics] tachometer -> server_metrics_export.jsonl: 6319 timestamps, dedup 6319 -> 6319 lines
22:11:45 INFO [done] bundle ready in 568.5s: aiperf=True traces=False metrics=True request_trace=False

(incomplete-21.parquet | rows: 21008645 | has timestamp_ns: True.)

Two adjacent observations, not fixed here: the scraper never writes final.parquet (no shutdown compaction), so ingest always reads the last incomplete-N shard; and every release asset up to v1.0.96 is the pre-#350 binary because release.yaml reuses the previous release's tachometer binaries when the merged PR does not touch Rust sources and the #350 rebuild failed at actions/checkout. This fallback keeps those deployments producing a metrics bundle until the release pipeline ships a current binary.

@nv-yna
nv-yna marked this pull request as ready for review September 11, 2026 23:40
@nv-yna
nv-yna merged commit 4a16414 into NVIDIA:main Sep 11, 2026
6 checks passed
ishandhanani added a commit that referenced this pull request Sep 13, 2026
Nine conflicting files, resolved by hand:

- The process exporter (#413) and the wider node-exporter collector set (#415)
  arrive as tachometer-stage exporter launches; on this branch the exporters
  are services. process-exporter is now a service kind (`type:
  process-exporter`, implied while tachometer runs, `placement.node: all`),
  host-native from the `configs/process-exporter` binary `make setup`
  installs (no container, no mounts), skipped with a warning when the binary
  is missing, container launch when a recipe declares a `container`. The
  group file is written by the kind's `prepare` hook; `ServiceKind` gains
  `host_native`, `prepare`, and `skip_reason`. node-exporter's built command
  carries the stat/vmstat/pressure/meminfo_numa/processes collectors and the
  widened vmstat field filter. The templates, the group YAML, and the host
  binary resolver move to `srtctl.services.exporters`; their tests follow.
- Load-window tachometer (#359): start/stop inside run_benchmark is kept;
  `stop_tachometer` now terminates through `ManagedProcess.terminate`, which
  signals the Slurm step (SIGTERM to the srun client would abort the step and
  SIGKILL the scraper). The scraper's `terminate_timeout` is the recipe's
  `shutdown_grace_secs`; the 90 s module constant is gone.
- Ingest timestamp fallback (#414) is taken from main wholesale (it also
  offers `--start-ns`); this branch's own fallback is dropped, its dedup
  module and docstring edits re-applied.
- CPU power telemetry (#410, #422): taken as is (power is frozen); the
  energy report runs before the S3 upload, which returns the URL only.
- Makefile: the `examples`/`golden-check` targets plus the cpu-power targets;
  the `recipes/`-based runner targets stay deleted. SUMMARY: both new pages,
  `analyzing.md` stays deleted. submit.py: the direct-host renderer import
  stays deleted; the arch helpers the cpu-power preflight uses are kept.
  telemetry.py: IPv6-safe host and the SGLang gateway metrics port together.

2355 tests on Python 3.10 and 3.13, lint, schema docs, 21 examples validated,
golden 574 identical / 0 mismatched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants