Repository navigation
[None][fix] Autotuner cache: merge instead of replace, atomic save - #14441
Conversation
|
/bot run --disable-fail-fast --add-multi-gpu-tests |
|
PR_Github #49880 Bot args parsing error: usage: /bot [-h] |
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #49887 [ run ] triggered by Bot. Commit: |
Two related fixes to AutoTunerProfilingCache (tensorrt_llm/_torch/autotuner.py)
so concurrent jobs sharing TLLM_AUTOTUNER_CACHE_PATH stop losing work
and the cache file is no longer corrupted by a writer killed mid-write.
1. Merge, do not replace, the per-rank dict on save.
current_cache[f"rank_{rank}"] = serialized_rank_cache was wiping
every other concurrent writer's rank_{rank} contribution and any
preexisting on-disk entries the current process did not load.
Replaced with the same setdefault+update pattern already used for
the shared dict. Verified by a 4-job synthetic stress: 0 of 3
lost-update rounds after the fix vs 3 of 3 before.
2. Move locking off the data file and replace truncate+json.dump
with write-to-tmp + fsync + os.rename.
The previous code held fcntl.flock(LOCK_EX) on the data file in
'a+' mode and did seek+read+truncate+json.dump in-place. Two real
failure modes:
* a writer killed mid-json.dump (preempt / OOM / scancel) leaves
a partially-written file and every subsequent reader fails with
JSONDecodeError. Observed on a 60-job stress with payload ~30MB.
* O_APPEND from 'a+' plus truncate semantics are filesystem-quirky
and make the failure unrecoverable across nodes.
New design: fcntl.lockf(LOCK_EX) on a SEPARATE persistent lockfile,
then write the body to <name>.tmp.<pid>.<ns>, fsync, then os.rename.
A killed writer leaves only a discardable tmp file; cache.json is
never partially written. fcntl.lockf is the POSIX byte-range variant,
which is more portable across Lustre / NFS than flock(2).
Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
1c510db to
1c05254
Compare
|
PR_Github #49887 [ run ] completed with state |
|
/bot run --disable-fail-fast --add-multi-gpu-test |
|
PR_Github #50157 [ run ] triggered by Bot. Commit: |
|
PR_Github #50157 [ run ] completed with state |
…VIDIA#14441) Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
…VIDIA#14441) Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
…VIDIA#14441) Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com>
…VIDIA#14441) Signed-off-by: Yukun He <23156053+hyukn@users.noreply.github.com> (cherry picked from commit 335a418)
Description
Two related fixes to
AutoTunerProfilingCache(tensorrt_llm/_torch/autotuner.py) so concurrent jobs sharingTLLM_AUTOTUNER_CACHE_PATHstop losing work and the cache file is no longer corrupted by a writer killed mid-write.1. Merge, do not replace, the per-rank dict on save
current_cache[f"rank_{rank}"] = serialized_rank_cachewas wiping every other concurrent writer'srank_{rank}contribution and any preexisting on-disk entries the current process did not load.Replaced with the same
setdefault(...).update(...)pattern already used for the shared dict.Verified by a 4-job synthetic stress (
cache-race-multi-job/): 0 of 3 lost-update rounds after fix vs 3 of 3 before.2. Move locking off the data file and replace truncate+json.dump with write-tmp + fsync + os.rename
The previous code held
fcntl.flock(LOCK_EX)on the data file in'a+'mode and didseek + read + truncate + json.dumpin-place. Two real failure modes:json.dump(preempt / OOM / scancel / node failure) leaves a partially-written file and every subsequent reader fails withJSONDecodeError. Observed on a 60-job stress with a payload ~30 MB.O_APPENDfrom'a+'plustruncatesemantics are filesystem-quirky and make the failure unrecoverable across nodes.New design:
fcntl.lockf(LOCK_EX)on a separate persistent lockfile next to the data file (POSIX byte-range lock; more portable across Lustre / NFS than BSDflock(2)). Lockfile is never unlinked; the kernel releases the lock on fd close including process death, so no stale-lock cleanup path is needed.<name>.tmp.<pid>.<ns>,fsync'd, and thenos.rename'd. A killed writer leaves only a discardable tmp file; the data file is never partially written, so concurrent readers always observe a fully-formed JSON document.Relation to PR #14458
The
TuningConfig.exclude_from_cacheflag and single-(runner, tactic) profile shortcut for JIT-driver ops were originally bundled here. They are now split into PR #14458, opened independently againstfeat/deepseek_v4. The two PRs do not depend on each other.Test Coverage
Validated end-to-end on lyris with the following stress harnesses (in this repo under
cache-race-multi-job/andcache-race-e2e-v3lite/):autotune()so save_cache calls all overlap. Each writer tunes a unique op-name + payload so lost entries are detectable by counting. Before fix: 3 of 3 rounds dropped 3 of 4 writers' contributions; after fix: 0 of 3 rounds drop any writer.'a+' + flockdesign and the lock disabled, the file corrupts within the first 10 iterations (JSONDecodeError: Extra data ...). With the new design (separate lockfile + atomic rename) the file remains valid JSON across 9 600 RMW ops.trtllm-bench throughput --tp 4runs sharing oneTLLM_AUTOTUNER_CACHE_PATH. Every writer's contribution survives in the on-disk file; no crashes.No new unit tests added in this PR (the existing
tests/unittest/_torch/misc/test_autotuner.pyhas zero coverage of save_cache / load_cache — adding multi-writer coverage is a worthwhile follow-up but out of scope for this fix).PR Checklist
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.