Skip to content

storage: a zarr cache entry counts only once its write has finished - #16

Open
danohu wants to merge 4 commits into
mainfrom
zarr-cache-complete-marker
Open

danohu wants to merge 4 commits into
mainfrom
zarr-cache-complete-marker

Conversation

@danohu

@danohu danohu commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

For zarr files, set up a .complete marker file to check when data is fully written.

This replaces relying on zarr.json -- that doesn't work because zarr.json is the first to be written. This might happen from a failed or aborted calculation, or because multiple processes are trying to write the same file (e.g. running 2 countries which share a tile)

A partially-written zarr might be not cause any warning signs later, since it might be techncially valid in spite of missing data.

Now an empty .complete marker is written beside the zarr after its last chunk, and the entry counts as cached only once the marker exists. A zarr found without its marker is a write that was killed part-way or is still running: serialize raises FileExistsError and leaves it for someone to resolve by hand, usually by deleting the zarr, rather than writing over it and risking a race with a live writer.

Entries written before this change have no marker, so each raises once until deleted.

Secondary Change -- allow unit tests to run without a .env

Questions for reviewers

What to do when we hit an incomplete zarr? options:

  • [what this PR does] raise an exception: conservative, potentially annoying
  • delete the partial data: works for detritus from older failed/aborted runs, doesn't deal well with race conditions

This will break existing storage, requiring either a bump of the appropriate versions or to delete the zarr folders. Maybe merge when we're bumping the version anyway?

For zarr files, set up a .complete marker file to check when data is
fully written.

This replaces relying on zarr.json -- that doesn't work because
zarr.json is the first to be written. This might happen from a failed or
aborted calculation, or because multiple processes are trying to write
the same file (e.g. running 2 countries which share a tile)

A partially-written zarr might be not cause any warning signs later,
since it might be techncially valid in spite of missing data.

Now an empty .complete marker is written beside the zarr after its last chunk, and the
entry counts as cached only once the marker exists. A zarr found without its marker is a
write that was killed part-way or is still running: serialize raises FileExistsError and
leaves it for someone to resolve by hand, usually by deleting the zarr, rather than
writing over it and risking a race with a live writer.

Entries written before this change have no marker, so each raises once until deleted.
@danohu
danohu marked this pull request as draft October 15, 2026 18:38
An autouse fixture in jdluc/__tests__/conftest.py stubs Config.from_dot_env
for every test not marked integration. Each field gets its type's empty value,
with two dask workers, so a unit test that reaches for a storage root fails
instead of touching a bucket. Integration tests still read the real .env.

This replaces the fixture storage_test carried for itself, so the next unit
test that writes a zarr needs no copy of it, and a new Config field needs no
edit to the tests.
@danohu
danohu marked this pull request as ready for review October 15, 2026 19:35
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.

1 participant