fix(ci): use per-call temp dir in DNA templating to fix race-condition flake#857
Conversation
…n flake
The Holochain DNA templating helper inside `read_and_template_holochain_dna`
used `temp_dir().join(source_language_hash)` as its working directory. That
meant every templating call against the same source language landed on the
same `/tmp/<hash>/` path, with the top of the function wiping that path via
`fs::remove_dir_all` before recreating it.
Two integration tests in `tests/js/tests/neighbourhood.ts` both call
`languages.applyTemplateAndPublish(DIFF_SYNC_OFFICIAL, ...)` against the
shared `perspective-diff-sync` hash —
- "can be created by Alice and joined by Bob"
- "shared link created by Alice received by Bob"
— and on the busy CircleCI workers the second call regularly entered the
function while the first was still mid-pipeline (between `unpack_happ`
and `pack_happ`). The second call's `fs::remove_dir_all` wiped the
first's working directory, and the first's eventual `pack_happ` blew up
with:
Failed to pack hApp: ffs::IoError at path
'/tmp/QmzSY…/happ/happ.yaml': No such file or directory (os error 2)
That's been the long-standing flake on `integration-tests-js`.
Switch to a per-call UUID-suffixed temp dir
(`<temp_dir>/<source_hash>-<uuid_v4>/`). Concurrent templates can no
longer race on the same path, the existing end-of-function cleanup still
removes the per-call dir, and an earlier-failing call only leaves a
uniquely-named orphan rather than poisoning the next call. `uuid` is
already a workspace dependency (`uuid = "1.3.0"`).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesDNA Templating Concurrency Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
read_and_template_holochain_dna(inrust-executor/src/languages/mod.rs) usedtemp_dir().join(source_language_hash)as its working directory, so every templating call against the same source language landed on the same/tmp/<hash>/path. The top of the function wipes that path viafs::remove_dir_allbefore recreating it — which means concurrent calls against the same source language race each other.Two integration tests in
tests/js/tests/neighbourhood.tsboth calllanguages.applyTemplateAndPublish(DIFF_SYNC_OFFICIAL, ...)against the sharedperspective-diff-synchash:can be created by Alice and joined by Bobshared link created by Alice received by BobOn busy CircleCI workers the second call regularly enters the function while the first is still mid-pipeline (between
unpack_happandpack_happ). The second call'sfs::remove_dir_allwipes the first's working directory, and the first's eventualpack_happblows up with:This has been the long-standing flake on
integration-tests-js. It surfaced again under the #837 / #842 stack and has been recurring across CI runs.Fix
Switch to a per-call UUID-suffixed temp dir:
fs::remove_dir_all(&temp_templating_path)still removes the per-call dir on the happy path.uuidis already a workspace dependency (uuid = "1.3.0").Test plan
cargo check -p ad4m-executor --libcleancargo fmt --checkcleanSummary by CodeRabbit