fix: gate fused sync fast paths on full store sync capability; wrappers forward it - #257
Open
d-v-b wants to merge 1 commit into
Open
fix: gate fused sync fast paths on full store sync capability; wrappers forward it#257d-v-b wants to merge 1 commit into
d-v-b wants to merge 1 commit into
Conversation
d-v-b
force-pushed
the
fix/store-sync-capability-wrappers
branch
from
July 29, 2026 11:46
2d73aa3 to
3247035
Compare
Owner
Author
|
🤖 AI text below 🤖 Merge-order note: this branch has a small textual conflict with #254 in |
d-v-b
force-pushed
the
fix/store-sync-capability-wrappers
branch
from
July 29, 2026 13:29
3247035 to
186c392
Compare
7 tasks
d-v-b
force-pushed
the
fix/store-sync-capability-wrappers
branch
3 times, most recently
from
July 30, 2026 07:09
a9b0e73 to
c3dfafc
Compare
d-v-b
changed the title
d-v-b
force-pushed
the
fix/store-sync-capability-wrappers
branch
from
July 30, 2026 07:25
c3dfafc to
0227efd
Compare
…rs forward it The fused write gate checked only SupportsSetSync, but write_sync also needs get_sync (partial-chunk read-modify-write) and delete_sync (all-fill chunk cleanup): a set-sync-only store passed the gate, wrote some chunks, then died mid-batch with TypeError. And WrapperStore forwarded no *_sync method, so every wrapped store (e.g. LatencyStore) silently lost the sync fast path — latency benchmarks measured the async fallback while claiming to measure the fused sync path. Both gates now consult _store_supports_sync_io: structural membership in SupportsSyncStore (the full get/set/delete sync surface) combined with a per-instance _supports_sync_io opt-out (absent means capable). This is a private, interim convention pending a formal sync/async store architecture — the store-side twin of the codec-side _sync_capable convention from zarr-developers#4179 — deliberately not new public API. WrapperStore delegates the three sync methods and forwards the wrapped store's capability, so wrapping a sync store keeps the fast path and wrapping an async-only store falls back cleanly; LoggingStore logs the delegated sync calls. LatencyStore fixes: sync reads/writes now sleep the configured latency on the worker thread; get_ranges/get_partial_values route through the latency-injecting get instead of bypassing the wrapper; _with_store passes the raw (loc, scale) latency config instead of a single sampled float, so derived stores keep the distribution. Assisted-by: ClaudeCode:claude-fable-5
d-v-b
force-pushed
the
fix/store-sync-capability-wrappers
branch
from
July 30, 2026 07:30
0227efd to
029e05a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖
Reworked per the discussion on zarr-developers#4206: no public API change. The capability decision is now a private, interim convention —
zarr.abc.store._store_supports_sync_io(store)= structuralSupportsSyncStoremembership ∧ a per-instance_supports_sync_ioopt-out (absent means capable) — the store-side twin of the codec-side_sync_capableconvention from zarr-developers#4179, with the same deletion path when a formal sync/async store architecture lands.Fixes (unchanged from before): full-surface write gating (set-sync-only stores get a clean async fallback instead of a mid-batch
TypeError),WrapperStoresync delegation + capability forwarding (wrapped sync stores keep the fused fast path; wrappers of async-only stores fall back),LoggingStoresync logging, and the threeLatencyStorefixes (sync latency injection,get_ranges/get_partial_valueslatency,_with_storepreserving stochastic configs).All behavior tests unchanged; 1296 passed across store + pipeline suites; mypy clean.
🤖 Generated with Claude Code