Skip to content

fix: bound memory used by resize chunk cleanup - #4344

Open
2160039878-cyber wants to merge 1 commit into
zarr-developers:mainfrom
2160039878-cyber:fix/bounded-resize-memory
Open

2160039878-cyber wants to merge 1 commit into
zarr-developers:mainfrom
2160039878-cyber:fix/bounded-resize-memory

Conversation

@2160039878-cyber

Copy link
Copy Markdown

Summary

Addresses the remaining shrinking and mixed growing/shrinking cases in #3650.

Resize cleanup currently materializes both complete chunk grids and schedules every deletion at once. This generates only the out-of-bounds coordinates and deletes them in bounded batches, so auxiliary memory depends on the number of dimensions and the batch size rather than the total grid size.

The coordinate generator partitions the removed region by its first out-of-bounds axis. Integer iteration avoids itertools.product caching very large axes. Batches reuse concurrent_map, respect async.concurrency, and contain at most 1000 deletions when that setting is None. The existing default for delete_outside_chunks is preserved.

For reviewers

I would particularly appreciate a review of the disjoint coordinate regions for mixed resizing and the batching behavior with unlimited configured concurrency. The tests compare coordinates against set difference, check for duplicates and pending-task bounds, and verify retained chunk bytes across v2/v3, regular/rectilinear, and sharded grids. Deletion failures still propagate before metadata is updated; already-deleted chunks are not rolled back.

Validation on commit 3a8ab604fca937a35de30be6cdcb4b3d4041e606:

  • Local Windows/Python 3.12 full suite, excluding benchmarks as in the project test command: 8635 passed, 1338 skipped, 4 xfailed. The focused resize/append selection passed 83 tests.
  • Test workflow: all 15 jobs successful, including Linux/Windows/macOS, Python 3.12–3.14, dependency variants, doctests, and benchmark smoke tests.
  • Slow Hypothesis: 15 passed, 3 skipped, using the nightly profile.
  • Lint, changelog, Docs, Wheels, and zarr-http-server workflows succeeded on the fork.
  • The regression test resizes a sparse (2**40, 2) chunk grid while issuing only two deletions. Local statement coverage is 100% for the coordinate generator and _resize.

Verification limits: GPU tests require the upstream runner. Codecov uploads on the fork were rejected because a token was required. The non-blocking docs type-link check reports the same 83 unlinked types across 44 pages on this commit and the unmodified baseline.

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.

AI assistance: OpenAI Codex was used for the implementation, tests, and drafting this description. I have personally reviewed and understood the code changes.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions — not applicable; the new helper is private.
  • New/modified features documented in docs/user-guide/*.md — not applicable; this is a cleanup-memory bug fix.
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed — seven fork workflows passed; upstream PR checks and GPU validation are pending.
  • Test coverage is 100% (Codecov passes) — the two changed functions have 100% local statement coverage; Codecov acceptance is pending.

Co-Authored-By: OpenAI Codex <noreply@openai.com>
@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.34%. Comparing base (fb7ad35) to head (3a8ab60).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4344   +/-   ##
=======================================
  Coverage   94.34%   94.34%           
=======================================
  Files          92       92           
  Lines       12935    12950   +15     
=======================================
+ Hits        12203    12218   +15     
  Misses        732      732           
Files with missing lines Coverage Δ
src/zarr/core/array.py 98.11% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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