Skip to content

Conversation

jhamman
Copy link
Member

@jhamman jhamman commented Oct 15, 2025

This PR updates the default async.concurrency setting in Zarr from 10 to 64. We've repeatedly seen (example 1, example 2) that Zarr performs much better at larger concurrency settings and the default value of 10 was a hold over from the very early experiments with Zarr-Python 3.

I talked this over with @d-v-b and @normanrz here at the Zarr Summit and both were cool with the change 😎

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)

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Oct 15, 2025
Copy link
Contributor

@d-v-b d-v-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@d-v-b d-v-b enabled auto-merge (squash) October 15, 2025 10:08
@d-v-b d-v-b merged commit 14b372c into zarr-developers:main Oct 15, 2025
31 checks passed
@rabernat
Copy link
Contributor

We should also make this a global limit, not scoped to each chunk request.

@dcherian
Copy link
Contributor

We should also make this a global limit, not scoped to each chunk request.

+1000. In fact i think we should revert this until that change is made; because it's going to cause errors with dask as we've learned with Icechunk.

@d-v-b
Copy link
Contributor

d-v-b commented Oct 15, 2025

We should also make this a global limit, not scoped to each chunk request.

+1000. In fact i think we should revert this until that change is made; because it's going to cause errors with dask as we've learned with Icechunk.

sounds good to me. @jhamman do you want to revert?

@jhamman
Copy link
Member Author

jhamman commented Oct 15, 2025

I don't exactly understand what the scope of the global limit is. My preference would be to keep this change in AND document a best practice for using Zarr concurrency with Dask AND design a proper global semaphore for Zarr. I would happily lead the documentation effort but the global design is unclear to me at the moment.

@dstansby
Copy link
Contributor

Could this get some documentation and a changelog entry too? Recently I was quite confused by the different concurrency options when I wanted to force zarr-python to run with no concurrency, so changing/improving the default seems like a good oppurtunity to add some short docs on what this option affects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants