Add portable Retriever nightly launcher#2358
Conversation
Greptile SummaryThis PR introduces a portable nightly benchmark launcher (
|
| Filename | Overview |
|---|---|
| ops/retriever-nightly/run-nightly.sh | New 577-line bash launcher; well-structured two-phase design with proper quoting, flock guards, config-file ownership validation, and bounded subprocess execution. |
| nemo_retriever/src/nemo_retriever/harness/runsets.py | Adds _run_prepared_benchmark_isolated with spawn-context subprocess, poll-based receive with 6-hour timeout, bounded 30-second post-result join, and correct message-first ordering when exit code is non-zero; run_runfiles unconditionally enables isolation for non-dry-run executions. |
| nemo_retriever/src/nemo_retriever/harness/vidore_access.py | New module probing one byte per HuggingFace partition with streaming range requests; Authorization header forwarded via raw requests.get with allow_redirects=True may leak the Bearer token to CDN hosts on 307 redirects. |
| nemo_retriever/src/nemo_retriever/harness/artifacts.py | Adds working_tree_dirty(), expands commit SHA regex to 64 chars for SHA-256 compat, and switches last_commit to return the full SHA rather than the 7-char short form. |
| nemo_retriever/src/nemo_retriever/harness/cli.py | Adds check-vidore-access CLI command delegating to check_vidore_access; exits with EXIT_MISSING_INPUT on VidoreAccessError. |
| nemo_retriever/tests/test_harness_nightly_launcher.py | New launcher integration tests using a fake uv binary and real git init fixtures; covers dirty-checkout labels, config-file security checks, Slack dedup, and access-preflight skip on --dry-run. |
| nemo_retriever/tests/test_harness_nightly_git_selection.py | New tests for the select_checkout_and_run Git selection logic; covers local refs, remote fetch, detached-worktree creation, dirty-worktree rejection, and lock-contention behaviour. |
| nemo_retriever/tests/test_harness_vidore_access.py | New unit tests for check_vidore_access using injected fake api and get callables; covers missing-token, bad-status, empty-response, and missing-partition failure paths. |
| nemo_retriever/tests/test_harness_runfiles.py | Extended with isolation-path tests: verifies that isolated child failures don't terminate the session, that the session summary records isolate_runs, and that post-result process-hang is handled gracefully. |
Sequence Diagram
sequenceDiagram
participant User
participant Launcher as run-nightly.sh (phase 1)
participant Checkout as Selected Checkout
participant Launcher2 as run-nightly.sh (phase 2)
participant Harness as retriever harness run-files
participant Child as Isolated Child Process
participant Slack as Slack Webhook
User->>Launcher: ./run-nightly.sh [--ref REF] [runfiles...]
Launcher->>Launcher: load_config_defaults (nightly.env, mode 600)
alt --ref provided
Launcher->>Checkout: git fetch + worktree add (detached)
else bare command
Launcher->>Checkout: use current checkout (allow dirty)
end
Launcher->>Launcher2: "RETRIEVER_SELECTED_CHECKOUT=... run-nightly.sh --check-vidore-access"
Launcher2->>Harness: retriever harness check-vidore-access
Harness-->>Launcher2: preflight result
Launcher2-->>Launcher: exit 0 / non-zero
alt preflight passed
Launcher->>Launcher2: "RETRIEVER_SELECTED_CHECKOUT=... run-nightly.sh [args]"
Launcher2->>Harness: retriever harness run-files --session-name ... [runfiles]
loop "each runfile (isolate_runs=True)"
Harness->>Child: spawn process (run_prepared_benchmark)
Child-->>Harness: pipe send (outcome, RunOutcome)
Child->>Child: cleanup (Ray, GPU) bounded 30s
end
Harness-->>Launcher2: session_summary.json + exit code
Launcher2->>Slack: retriever harness post-slack (once, atomic marker)
Slack-->>Launcher2: 200 OK
else preflight failed
Launcher->>User: log error, skip GPU work
end
Reviews (11): Last reviewed commit: "fix(harness): preserve received isolated..." | Re-trigger Greptile
4710243 to
51a8013
Compare
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
jperez999
left a comment
There was a problem hiding this comment.
Just take a look at that greptile comment I think it might be valid. Wouldn't want one hang to screw the entire nightly run.
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Signed-off-by: Jacob Ioffe <jioffe@nvidia.com>
Summary
--refthe explicit reproducible path: local refs run from clean detached worktrees, while remote refs such asupstream/mainare fetched first and fail closedworking_tree_dirtyin session summaries; dirty runs also writesource_worktree_status.txtand label Slack reports[LOCAL CHANGES]/datasets/nv-ingestmap, and a safe DeepGEMM warmup defaultnightly.env, which supplies only missing valuesSLACK_WEBHOOK_URLis present; omit it to disable Slack or pass--no-slackfor a functional testLead test: run the checked-out code
On a host with
/datasets/nv-ingest, export the supported secrets:Then run the current branch, including any local harness changes:
That is the complete development entry point. It writes artifacts under
/raid/$USER/retriever-nightly-artifactswhen/raid/$USERis writable, continues through child failures, writes one terminalsession_summary.json, and posts one terminal Slack report. It requires nosudo, systemd service, timer, or configuration file.To deliberately ignore local changes and run the newest clean upstream commit:
The same
--refinterface accepts another remote branch, local branch, tag, or full SHA.Optional functional checks
The launcher performs the ViDoRe access preflight before every real invocation, including a JP20-only canary, so
HF_TOKENis required for each real launcher run.Configuration contract
Exported variables work without a configuration file. If
<nightly-root>/.config/nemo-retriever/nightly/nightly.envexists, the launcher loads it after verifying that the invoking user owns it and its mode is600. Already-exported supported settings win; the file supplies only missing values. The launcher never requires that file and never discovers a repository.env.HF_TOKENSLACK_WEBHOOK_URLRETRIEVER_DATASET_PATHS/datasets/nv-ingestmap.Nonstandard dataset host
--dataset-pathsandRETRIEVER_DATASET_PATHStake the path to a YAML file, not a dataset directory. A JP20-only file may contain only:Run it with:
Batch mode starts local models and needs no model-provider API keys. The launcher defaults
VLLM_DEEP_GEMM_WARMUP=skipunless explicitly overridden.Daily latest-main workflow
The launcher remains a one-shot developer tool. For an operator-owned daily loop, run this command inside the documented
tmuxloop:Each invocation fetches
upstream/main, selects its exact commit in a clean detached worktree, runs access preflight, and then executes the suite. It never pulls, merges, or moves the invoking checkout.Validation
test_harness*.pytests passed after merging currentupstream/maingit diff --checkpassedretriever harness run-fileswith no isolation or timeout flags:success: true,exit_code: 0,dry_run: false,isolate_runs: true, two runsThe real run was performed immediately before commit
38e1a36bwith the exact isolation simplification diff; its artifact provenance correctly records the pre-commit checkout as dirty. The subsequentupstream/mainmerge atb3bbec8ddid not change the isolation implementation, and the merged tree passed the 125-test harness suite.