Skip to content

Commit 4f1ad9f

Browse files
committed
Use Store.get_many for whole-chunk reads in BatchedCodecPipeline
BatchedCodecPipeline.read now fetches the encoded bytes for an entire (non-sharded) read with a single Store.get_many call, instead of one Store.get per chunk. It drives get_many over all chunk keys, scatters the completion-ordered (index, buffer) results back into position, and feeds them to the per-batch decode path. This lets a store batch or coalesce the underlying reads (e.g. FsspecStore via cat_ranges, or a custom store such as virtualizarr's ManifestStore / icechunk's IcechunkStore that overrides get_many) regardless of codec_pipeline.batch_size, which still governs only decode batching. The sharding codec's partial-decode path is untouched, and stores without a specialized get_many fall back to the previous concurrent per-chunk gets.
1 parent 9e6eeae commit 4f1ad9f

3 files changed

Lines changed: 161 additions & 13 deletions

File tree

changes/1758.feature.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
`BatchedCodecPipeline` now fetches the encoded bytes for a whole (non-sharded)
2+
read with a single `Store.get_many` call spanning the entire request, instead of
3+
issuing one `Store.get` per chunk. This lets a store batch or coalesce the
4+
underlying reads — for example `FsspecStore` coalesces nearby chunk reads via
5+
`cat_ranges`, and a custom store (such as virtualizarr's `ManifestStore` or
6+
icechunk's `IcechunkStore`) can override `get_many` to merge reads that resolve
7+
into the same underlying object — independently of `codec_pipeline.batch_size`,
8+
which still governs only decode batching. The sharding codec's partial-decode
9+
path is unchanged, and stores without a specialized `get_many` fall back to the
10+
previous concurrent per-chunk behavior.

src/zarr/core/codec_pipeline.py

Lines changed: 103 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from dataclasses import dataclass, field
44
from itertools import batched, pairwise
5-
from typing import TYPE_CHECKING, Any
5+
from typing import TYPE_CHECKING, Any, cast
66
from warnings import warn
77

88
from zarr.abc.codec import (
@@ -23,14 +23,61 @@
2323
from zarr.registry import register_pipeline
2424

2525
if TYPE_CHECKING:
26-
from collections.abc import Iterable, Iterator
26+
from collections.abc import Iterable, Iterator, Mapping, Sequence
2727
from typing import Self
2828

29-
from zarr.abc.store import ByteGetter, ByteSetter
29+
from zarr.abc.store import ByteGetter, ByteSetter, Store
3030
from zarr.core.array_spec import ArraySpec
3131
from zarr.core.buffer import Buffer, BufferPrototype, NDBuffer
3232
from zarr.core.dtype.wrapper import TBaseDType, TBaseScalar, ZDType
3333
from zarr.core.metadata.v3 import ChunkGridMetadata
34+
from zarr.storage._common import StorePath
35+
36+
37+
def _bulk_store_keys(
38+
batch_info_list: list[tuple[ByteGetter, ArraySpec, SelectorTuple, SelectorTuple, bool]],
39+
) -> tuple[Store, list[str], BufferPrototype] | None:
40+
"""Return ``(store, keys, prototype)`` when a whole-chunk read batch can be
41+
served by a single :meth:`~zarr.abc.store.Store.get_many` call, else ``None``.
42+
43+
Eligible when the batch is non-empty and every chunk is backed by a
44+
``StorePath`` sharing one ``Store`` (compared by identity) and one buffer
45+
prototype. When it is not eligible (e.g. a mix of stores, or virtual byte
46+
getters used by the sharding codec) the caller should fall back to
47+
per-chunk fetching.
48+
"""
49+
# Local import to avoid a module-load import cycle (storage imports core).
50+
from zarr.storage._common import StorePath
51+
52+
if not batch_info_list:
53+
return None
54+
byte_getters = [byte_getter for byte_getter, *_ in batch_info_list]
55+
if not all(isinstance(bg, StorePath) for bg in byte_getters):
56+
return None
57+
store_paths = cast("list[StorePath]", byte_getters)
58+
store = store_paths[0].store
59+
prototype = batch_info_list[0][1].prototype
60+
if not all(sp.store is store for sp in store_paths):
61+
return None
62+
if not all(array_spec.prototype is prototype for _, array_spec, *_ in batch_info_list):
63+
return None
64+
return store, [sp.path for sp in store_paths], prototype
65+
66+
67+
async def _collect_get_many(
68+
store: Store, keys: Sequence[str], prototype: BufferPrototype
69+
) -> list[Buffer | None]:
70+
"""Drive ``Store.get_many`` for a set of whole-key reads and return the
71+
results as a positional list aligned to ``keys`` (``None`` for absent keys).
72+
73+
``get_many`` yields ``(request_index, Buffer | None)`` batches in completion
74+
order, so we scatter each result back to its input position.
75+
"""
76+
out: list[Buffer | None] = [None] * len(keys)
77+
async for batch in store.get_many(keys, prototype=prototype):
78+
for index, buffer in batch:
79+
out[index] = buffer
80+
return out
3481

3582

3683
def _unzip2[T, U](iterable: Iterable[tuple[T, U]]) -> tuple[list[T], list[U]]:
@@ -353,11 +400,47 @@ async def encode_partial_batch(
353400
assert isinstance(self.array_bytes_codec, ArrayBytesCodecPartialEncodeMixin)
354401
await self.array_bytes_codec.encode_partial(batch_info)
355402

403+
async def _get_chunk_bytes_batch(
404+
self,
405+
batch_info_list: list[tuple[ByteGetter, ArraySpec, SelectorTuple, SelectorTuple, bool]],
406+
prefetched: Mapping[str, Buffer | None] | None = None,
407+
) -> list[Buffer | None]:
408+
"""Fetch the whole encoded bytes for each chunk in a batch.
409+
410+
If ``prefetched`` is supplied (a mapping of store key to already-read
411+
``Buffer``, as produced by :meth:`read`), the bytes are taken from it
412+
directly. Otherwise, when every chunk is backed by a ``StorePath`` over
413+
a single common ``Store`` and buffer prototype, the reads are handed to
414+
the store as one :meth:`~zarr.abc.store.Store.get_many` call so a
415+
backend that can batch or coalesce object reads gets the chance to do
416+
so. Failing that, it falls back to fetching each chunk concurrently
417+
with :meth:`~zarr.abc.store.ByteGetter.get`, matching prior behavior.
418+
"""
419+
if prefetched is not None:
420+
# read() already fetched these keys in one get_many call.
421+
store_paths = cast("list[StorePath]", [bg for bg, *_ in batch_info_list])
422+
return [prefetched.get(sp.path) for sp in store_paths]
423+
424+
plan = _bulk_store_keys(batch_info_list)
425+
if plan is not None:
426+
store, keys, prototype = plan
427+
return await _collect_get_many(store, keys, prototype)
428+
429+
return await concurrent_map(
430+
[
431+
(byte_getter, array_spec.prototype)
432+
for byte_getter, array_spec, *_ in batch_info_list
433+
],
434+
lambda byte_getter, prototype: byte_getter.get(prototype),
435+
config.get("async.concurrency"),
436+
)
437+
356438
async def read_batch(
357439
self,
358440
batch_info: Iterable[tuple[ByteGetter, ArraySpec, SelectorTuple, SelectorTuple, bool]],
359441
out: NDBuffer,
360442
drop_axes: tuple[int, ...] = (),
443+
prefetched: Mapping[str, Buffer | None] | None = None,
361444
) -> tuple[GetResult, ...]:
362445
results: list[GetResult] = []
363446
if self.supports_partial_decode:
@@ -381,14 +464,7 @@ async def read_batch(
381464
results.append(GetResult(status="missing"))
382465
else:
383466
batch_info_list = list(batch_info)
384-
chunk_bytes_batch = await concurrent_map(
385-
[
386-
(byte_getter, array_spec.prototype)
387-
for byte_getter, array_spec, *_ in batch_info_list
388-
],
389-
lambda byte_getter, prototype: byte_getter.get(prototype),
390-
config.get("async.concurrency"),
391-
)
467+
chunk_bytes_batch = await self._get_chunk_bytes_batch(batch_info_list, prefetched)
392468
chunk_array_batch = await self.decode_batch(
393469
[
394470
(chunk_bytes, chunk_spec)
@@ -590,9 +666,24 @@ async def read(
590666
out: NDBuffer,
591667
drop_axes: tuple[int, ...] = (),
592668
) -> tuple[GetResult, ...]:
669+
batch_info = list(batch_info)
670+
# For whole-chunk reads (not partial/sharded decode), fetch the encoded
671+
# bytes for the entire request in a single Store.get_many call. This
672+
# lets the store batch or coalesce the reads regardless of
673+
# codec_pipeline.batch_size (which only governs decode batching), and
674+
# restores the bulk-fetch behavior of the v2 getitems Store API. The
675+
# per-batch read_batch calls then read their bytes from this mapping.
676+
prefetched: Mapping[str, Buffer | None] | None = None
677+
if not self.supports_partial_decode:
678+
plan = _bulk_store_keys(batch_info)
679+
if plan is not None:
680+
store, keys, prototype = plan
681+
values = await _collect_get_many(store, keys, prototype)
682+
prefetched = dict(zip(keys, values, strict=True))
683+
593684
batch_results = await concurrent_map(
594685
[
595-
(single_batch_info, out, drop_axes)
686+
(single_batch_info, out, drop_axes, prefetched)
596687
for single_batch_info in batched(batch_info, self.batch_size)
597688
],
598689
self.read_batch,

tests/test_codec_pipeline.py

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@
1919
from zarr.storage import MemoryStore
2020

2121
if TYPE_CHECKING:
22-
from collections.abc import Callable
22+
from collections.abc import AsyncIterator, Callable, Sequence
2323

2424
from zarr.abc.codec import Codec
25+
from zarr.abc.store import ByteRequest
26+
from zarr.core.buffer import Buffer, BufferPrototype
2527

2628

2729
@pytest.mark.parametrize(
@@ -195,3 +197,48 @@ def test_codecs_from_list_outcome_matches_order_rules(labels: list[str]) -> None
195197
aa, _ab, bb = codecs_from_list(codecs)
196198
assert labels.count(_AA) == len(aa)
197199
assert labels.count(_BB) == len(bb)
200+
201+
202+
async def test_read_uses_bulk_get_many() -> None:
203+
"""The pipeline should fetch a whole multi-chunk read with a single
204+
``Store.get_many`` call (spanning the entire request, independent of
205+
``codec_pipeline.batch_size``), rather than one ``get`` per chunk."""
206+
store = MemoryStore()
207+
arr = zarr.create_array(store, shape=(20,), chunks=(5,), dtype="int64") # 4 chunks
208+
arr[:] = np.arange(20)
209+
210+
calls: dict[str, int] = {"get_many": 0, "requests": 0}
211+
orig_get_many = store.get_many
212+
213+
# get_many is an async generator, so the spy is a sync function returning
214+
# the underlying async iterator; count at call time.
215+
def spy_get_many(
216+
requests: Sequence[tuple[str, ByteRequest | None] | str],
217+
*,
218+
prototype: BufferPrototype,
219+
) -> AsyncIterator[Sequence[tuple[int, Buffer | None]]]:
220+
requests = list(requests)
221+
calls["get_many"] += 1
222+
calls["requests"] += len(requests)
223+
return orig_get_many(requests, prototype=prototype)
224+
225+
store.get_many = spy_get_many # type: ignore[method-assign]
226+
227+
result = arr[:]
228+
np.testing.assert_array_equal(result, np.arange(20))
229+
# one bulk call covering all four chunks
230+
assert calls["get_many"] == 1
231+
assert calls["requests"] == 4
232+
233+
234+
async def test_read_bulk_handles_missing_chunks() -> None:
235+
"""A bulk read where some chunks were never written must still fill those
236+
positions with the fill value (get_many reports missing keys as None)."""
237+
store = MemoryStore()
238+
arr = zarr.open_array(store, mode="w", shape=(20,), chunks=(5,), dtype="int64", fill_value=-1)
239+
arr[0:5] = 7 # write only the first chunk; the other three are missing
240+
241+
result = arr[:]
242+
expected = np.full(20, -1, dtype="int64")
243+
expected[0:5] = 7
244+
np.testing.assert_array_equal(result, expected)

0 commit comments

Comments
 (0)