fix(store): relocate LocalStore temporary files to configurable tmp_dir

[omar-merhebi]

Summary

LocalStore now writes temporary files to a dedicated tmp_dir rather than directly inside the store path. Previously, _atomic_write wrote temp files ending in .partial inside the store which triggered ZarrUserWarning during concurrent reading and listing of the store.

The location defaults to the system temporary directory (via tempfile.gettempdir()), and is overridable via the tmp_dir argument to LocalStore or globally via the store.local.tmp_dir config option (env var ZARR_STORE__LOCAL__TMP_DIR). The temporary directory must be on the same filesystem as the store, otherwise _atomic_write will raise OSError with EXDEV. The pre-existing try/except now also has a specific catch for EXDEV and re-raises, informing the user to change the tmp_dir value to a location on the same filesystem rather than emitting a raw OSError. Documented this new option in config.md and storage.md. Fixes #4161.

For reviewers

I added a specific catch in the try/except block of _atomic_write() that specifically catches OSError caused by EXDEV and instructs the user to pick a temporary directory on the same filesystem if the atomic write fails. Happy to modify the message, change logic, or remove it entirely if preferred. I also verified that errno.EXDEV is the errno that is raised on both Unix and Windows. Used the convention of naming the config arg and vars as tmp_dir to copy the old tmp_path variable in _atomic_write(). Also happy to change that naming scheme if preferable. I specifically avoided shutil.move() here to keep LocalStore atomic (shutil.move() attempts os.rename() and falls back to copying if it fails). This follows the design discussed in #4161.

Author attestation

• I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.md
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[omar-merhebi]

Flagging this before anyone digs into the failing CIs

Both failures have the same root cause: temps default to the system temp dir, which is on a different filesystem than the store, so the os.replace() in _atomic_write() raises EXDEV. The new error handling is catching and re-raising it as designed, but it's happening on ordinary writes.

RTD (Linux): temp dir /tmp vs the checkout volume under /home/docs/checkouts/.... is leading to EXDEV on all the code blocks that write LocalStore. So /tmp is set up as a different filesystem.

Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/zarr/envs/4173/lib/python3.12/site-packages/zarr/storage/_local.py", line 78, in _atomic_write
    tmp_path.replace(path)
  File "/home/docs/.asdf/installs/python/3.12.13/lib/python3.12/pathlib.py", line 1376, in replace
    os.replace(self, target)
OSError: [Errno 18] Invalid cross-device link: '/tmp/7437a6df7e014a488d0b2fb54780f3c7.partial' -> 'data/example-1.zarr/zarr.json'

Windows (windows-latest, py3.14): the temp dir is on C: (C:\Users\...\Temp) and the checkout is on D:\. The OS itself reports this with WinError 17: OSError: [WinError 17] The system cannot move the file to a different disk drive:

Traceback (most recent call last):
  File "D:\a\zarr-python\zarr-python\src\zarr\storage\_local.py", line 78, in _atomic_write
    tmp_path.replace(path)
  File "C:\hostedtoolcache\windows\Python\3.14.6\x64\Lib\pathlib\__init__.py", line 1091, in replace
    os.replace(self, target)
OSError: [WinError 17] The system cannot move the file to a different disk drive: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\7a611477900f439a92b4a924519079c4.partial' -> 'example_from_array.zarr\\zarr.json'

ubuntu-latest: This passes because the Ubuntu runners keep temp on the checkout's disk so it's the same filesystem on those runners and it works. Ubuntu moved to using tmpfs by default on Ubuntu 24.10, prior to that /tmp was on the same filesystem as the store. The github runner is using Ubuntu 24.04.4, so it predates this change. Note that tmpfs is just a default. Users can customize how they set up their /tmp filesystem, so even Ubuntu versions prior to 24.10 may experience this issue (such as RTD Ubuntu 22.04 above).

Therefore, any setup where a store isn't on the temp dir's filesystem (e.g., a tmpfs /tmp directory, a separate data mount/drive, HPC, etc.) will see this. I think the fix would be to change the default to go to the store's filesystem either as a reserved subdirectory under the store root (would need list methods to skip it), or as a sibling to the store root (won't be in the store so listing methods are fine, but writes to an actual parent dir that the user might not expect us to use. Also doesn't fully eliminate the issues above, but does make them rarer). Happy to do whichever is preferred or implement other suggestions.

[d-v-b]

I didn't realize that containerized environments generally put temporary storage on a separate volume! that argues heavily against my original suggestion to use the default tmp location -- now it seems that this default is only sensible for writing and reading files, not moving them.

In light of that, I think it's probably better to write these temporary files in the same directory as the final key, as a default. Curious to hear what @mkitti thinks

[mkitti]

Having a temporary directory as an option is nice, but I think the default should be a directory that is likely to be the on same volume as the original data. Changing the default to a temporary directory that is not on the same volume and then erroring when attempting to do an atomic write would be a serious regression.

Better management of the warnings seems more appropriate if that is the primary complaint. The presence of .partial files while reading should produce some warning though as it seems likely that the reading client may receive a partially updated array. Perhaps we could just provide some explicit keyword to suppress that warning forcing the user explicitly acknowledge that caution is needed when attempting to interact with the data in this way.

[omar-merhebi]

@d-v-b @mkitti agreed on both counts. I would propose leaving the config option as nice to have but revert the default back to what main does which is a temp inside the store alongside the final key, and using the system temp dir, or any other location, as an opt-in users can specify.

That puts #4161 largely back to where we started though, so I would like to deliberate a bit on the warning before proceeding. I'm realizing _iter_members() warns on two cases, and I didn't realize the second case when I initially filed the issue.

Case 1: Rewriting an existing node's zarr.json.

A/
├── zarr.json
├── zarr.abc123.partial  <- currently being written
└── child/
    └── zarr.json

A.members() outputs ['child'] plus the warning Object at zarr.abc123.partial is not recognized .... This is the case I encountered where the warning is noisy. I use multiple workers to split up work in each store, and they each list the store on open while others are writing. This warning fires on each list so the number of warnings balloons with worker number.

Case 2: Creating a node.
There's a window during node creation where the node's directory exists but zarr.json doesn't yet. _put calls mkdir first before _atomic_write

A/
├── zarr.json
└── child/
    └── zarr.abc123.partial

Here A.members() outputs [], plus Object at child is not recognized .... The entire node gets dropped from the listing and the warning names the node itself rather than its temp file. It resolves once the rename finishes but any listing that happens inside that window comes back truncated and the warning attributes it to an unrecognized object rather than a concurrent write, so there's no way to tell a mid-creation node from actual junk. Same thing happens for arrays. Worth noting this window has nothing to do with where the temps go, mkdir has to happen before the rename regardless, since you can't rename a file into a directory that doesn't exist yet. Moving the temps elsewhere won't close it, because that needs the whole directory staged and not just the file. I can provide a repro script for both cases if you'd like to see it.

This changes what the fix would actually have to do. Filtering .partial out of the listing doesn't solve Case 2 and, @mkitti, that's also my hesitation with suppressing the warning. Suppressing the warning silences case 1, but it silences case 2 too; and case 2 is also what an actual corrupted store could look like, which is what the warning presumably exists for.

So I'd like to put the reserved in-root staging directory back on the table. I withdrew it before but I'm reopening it because I didn't notice case 2 and nothing else we have discussed fixes it. It would be excluded from list, list_prefix and list_dir by its one known path rather than by filename matching. It's the same filesystem by construction, so the atomic rename never crosses devices. Excluding one known path also means no filename matching, so there's no risk of hiding a real node whose name happens to end in .partial. And it's the only option I can think of that makes Case 2 fixable, since it gives you somewhere to stage a complete node directory and rename it into place. The v3 spec reserves __-prefixed names with __zarr specifically for core Zarr data. @d-v-b, you read that statement as an outlier and I do agree it's just there and unclarified, but @mkitti, I'm curious where you land on it.

If the staging directory works for you, I'd rework this PR onto it and that closes case 1 and #4161. If you'd rather not, I can reduce this PR to the config option with the default reverted and I can keep #4161 open. Case 2 should be its own issue either way in my opinion because that's a bigger change and is its own fix.

[mkitti]

Reading and writing from the same array is not a very well supported use case for raw Zarr, and we have few guardrails there. The warnings should be there to indicate this.

Suppressing the warnings should only be for those who really have thought about the I/O access patterns and concurrency issues. Warning suppression should draw scrutiny.

zarr.json modifications are particularly fraught because that could lead to corruption of the entire array.

These use pattern increasingly sounds like something that might require icechunk or something similar to maintain data integrity.

[omar-merhebi]

@mkitti fair enough, I'll take a look at icechunk. One clarification is that I'm not reading and writing the same array. Each worker works on a disjoint subtree, and no node is written by two workers nor is zarr.json modified concurrently. The only shared operation is listing the hierarchy on open.

So, with the warning staying as-is and the default reverting to what main already does, the only thing left in this PR is the configurable tmp_dir option. Do you think that's worth landing on its own or would you rather I close this? Happy to do the rework, just don't want to leave it open if it's not worth the maintenance.