fix(group): store a group's documents only; keep upgraded members as stored

Group writes (attributes, `update_attributes`, deleting a member,
`consolidate_metadata`, `create_hierarchy`) read each upgraded member of
the consolidated metadata again from its own document and stored its
upgrade. Those member writes raced concurrent deletions: two concurrent
`delitem`s on a consolidated group recreated a deleted array's metadata
document in most runs, which zarr 3.4.0 never did.

A group write now stores only the group's own documents and reads none.
Array metadata read from a document that had to be upgraded keeps that
document (`_stored_document`, set by `mark_upgraded`, replacing the
`_stored_document_upgraded` flag), and consolidated metadata stores such
a member as it was stored. Every reader of the consolidated metadata then
reads the member as upgraded again, and the array's own first chunk write
stores the upgrade of its current document (or refuses a changed chunk
grid) as before: the only write that upgrades a member document.

Removed: the refresh and adoption of consolidated members in
`save_metadata` (`_refresh_consolidated`, `_refresh_array`,
`_Refreshed`), and `Group.update_attributes_async`'s detour through
`save_metadata`. Tests pinning the refresh are replaced by tests that a
group write reads nothing and writes only the group's documents, stores
the member's consolidated copy byte for byte as stored, that concurrent
deletions leave no member, and that the first write through consolidated
metadata stores the member's upgrade or refuses a changed grid.

Assisted-by: ClaudeCode:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: d-v-b

changes/4334.bugfix.md

@@ -4,4 +4,4 @@ A chunk edge length is now always at least 1, while an array extent may be 0. Ev
 
 Stored metadata with a regular chunk size of 0 or JSON `false` (Zarr format 2 `chunks`, Zarr format 3 `regular` `chunk_shape`) is now read as one chunk spanning the axis (a multiple of the inner chunk size for sharded arrays, which previously failed to open). zarr-python wrote such sizes for arrays created with a zero-length axis until 3.4, and, from 2.18.7 to 3.2.1, for an explicit chunk size of 0 or `False` on an axis of any length, as in `chunks=(0,)` (those arrays could store no data). On a zero-length axis this opens silently, as before; appending to such an axis then stores chunks of size 1. On an axis of positive length it opens with a `ZarrUserWarning`, which says that the axis holds only the fill value and how to store valid metadata: `array.update_attributes({})`, then `zarr.consolidate_metadata` if the metadata is consolidated. JSON `true`, as zarr-python 3.0 and 3.2 wrote for a chunk size of `True`, is read as 1, silently, in the chunk grid (regular, and the explicit edges and run-length encoded sizes of a rectilinear one) and in the inner chunk shape of every sharding codec, nested or not. A stored rectilinear chunk grid whose edge lengths are integral JSON floats (`[[4.0, 2]]`), as zarr-python 3.2 wrote for float edges, is read with those edges as integers, silently; a float anywhere no release wrote one (a regular chunk shape, the inner chunk shape of a sharding codec, a run-length repeat count, an edge below 1) is rejected.
 
-Writing data to an array read from such metadata (other than an empty selection) first stores the upgrade of the metadata the store then holds, unless another writer has stored valid metadata since, so that other readers find the chunks written; in a `ZipStore` this adds a second entry for the metadata document, as every metadata update does. If the metadata the store then holds lays out chunks differently (another writer resized the array keeping the chunk size of 0, say), the write raises a `ValueError` asking to reopen the array, and stores nothing. Storing a group's metadata, which `zarr.consolidate_metadata` and every change to a group with consolidated metadata do, likewise first stores the upgrade of each such member's metadata and consolidates the member's metadata as the store then holds it.
+Writing data to an array read from such metadata (other than an empty selection) first stores the upgrade of the metadata the store then holds, unless another writer has stored valid metadata since, so that other readers find the chunks written; in a `ZipStore` this adds a second entry for the metadata document, as every metadata update does. If the metadata the store then holds lays out chunks differently (another writer resized the array keeping the chunk size of 0, say), the write raises a `ValueError` asking to reopen the array, and stores nothing. This is the only operation that stores the upgrade: a group's consolidated metadata keeps such an array's metadata as it was stored (`zarr.consolidate_metadata` copies it as the array's own document holds it), and changing a group reads and writes no metadata of its members.

src/zarr/core/array.py

@@ -1644,7 +1644,7 @@ async def _store_upgraded_document(self) -> None:
         upgrade. Storing the same upgrade twice is harmless, so concurrent callers need
         no coordination.
         """
-        if not self.metadata._stored_document_upgraded:
+        if self.metadata._stored_document is None:
             return
         zarr_format = self.metadata.zarr_format
         documents = await read_documents(self.store_path, ARRAY_DOCUMENTS[zarr_format])
@@ -1658,9 +1658,9 @@ async def _store_upgraded_document(self) -> None:
                     f"The metadata stored for the array at {str(self.store_path)!r} has "
                     "changed since this array was opened: reopen the array to write to it."
                 )
-            if current._stored_document_upgraded:
+            if current._stored_document is not None:
                 await upsert_metadata(self.store_path, current, documents)
-        object.__setattr__(self.metadata, "_stored_document_upgraded", False)
+        object.__setattr__(self.metadata, "_stored_document", None)
 
     async def _set_selection(
         self,

src/zarr/core/group.py

@@ -13,6 +13,7 @@
 
 import zarr.api.asynchronous as async_api
 from zarr.abc.metadata import Metadata
+from zarr.abc.store import Store, set_or_delete
 from zarr.core._info import GroupInfo
 from zarr.core._json import buffer_to_json_object, json_to_buffer
 from zarr.core.array import (
@@ -74,7 +75,6 @@
     )
     from typing import Any
 
-    from zarr.abc.store import Store
     from zarr.core.array_spec import ArrayConfigLike
     from zarr.core.buffer import Buffer, BufferPrototype
     from zarr.core.chunk_key_encodings import ChunkKeyEncodingLike
@@ -147,11 +147,16 @@ class ConsolidatedMetadata:
     must_understand: Literal[False] = False
 
     def to_dict(self) -> dict[str, JSON]:
+        """The consolidated metadata document. An array read from a stored document that
+        had to be upgraded is written as that document was stored: only the array's
+        own first chunk write stores its upgrade."""
         return {
             "kind": self.kind,
             "must_understand": self.must_understand,
             "metadata": {
                 k: v.to_dict()
+                if isinstance(v, GroupMetadata) or v._stored_document is None
+                else dict(v._stored_document)
                 for k, v in sorted(
                     self.flattened_metadata.items(),
                     key=lambda item: (
@@ -2115,7 +2120,9 @@ async def update_attributes_async(self, new_attributes: dict[str, Any]) -> Group
         new_metadata = replace(self.metadata, attributes=new_attributes)
 
         # Write new metadata
-        await save_metadata(self.store_path, new_metadata)
+        to_save = new_metadata.to_buffer_dict(default_buffer_prototype())
+        awaitables = [set_or_delete(self.store_path / key, value) for key, value in to_save.items()]
+        await asyncio.gather(*awaitables)
 
         async_group = replace(self._async_group, metadata=new_metadata)
         return replace(self, _async_group=async_group)
@@ -3032,8 +3039,6 @@ async def create_hierarchy(
     ```{'': GroupMetadata, 'a': GroupMetadata, 'b': Groupmetadata}```
 
     After input parsing, this function then creates all the nodes in the hierarchy concurrently.
-    The metadata of each node is stored as given: the consolidated metadata of a group is
-    stored as it is, without reading the documents of its members.
 
     Arrays and Groups are yielded in the order they are created. This order is not stable and
     should not be relied on.

src/zarr/core/metadata/io.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import asyncio
-from dataclasses import replace
 from enum import Enum
 from itertools import zip_longest
 from typing import TYPE_CHECKING, Final, NamedTuple
@@ -12,7 +11,6 @@
 from zarr.core.buffer.cpu import buffer_prototype as cpu_buffer_prototype
 from zarr.core.common import ZARR_JSON, ZARRAY_JSON, ZATTRS_JSON
 from zarr.core.metadata.upgrades import mark_upgraded, upgrade_array_document
-from zarr.core.metadata.v3 import RectilinearChunksDisabledError
 from zarr.errors import ArrayNotFoundError, ContainsArrayError
 from zarr.storage._common import StorePath, ensure_no_existing_node
 
@@ -134,87 +132,7 @@ def parse_stored_array(documents: Mapping[str, Buffer], zarr_format: ZarrFormat)
     else:
         raise ArrayNotFoundError(f"No Zarr format {zarr_format} array metadata document.")
     upgraded, readings = upgrade_array_document(stored, zarr_format)
-    return mark_upgraded(parse_array_metadata(dict(upgraded)), [None] * len(readings), None)
-
-
-async def _refresh_array(store_path: StorePath, member: ArrayMetadata) -> ArrayMetadata:
-    """The metadata of the array document stored at `store_path` as it now is, after
-    storing its upgrade if it needs one, for `member`, a member of consolidated metadata
-    that was read from a document that had to be upgraded; `member` itself if no array
-    document that can be read is stored there (it was deleted, or replaced by a group)."""
-    documents = await read_documents(store_path, ARRAY_DOCUMENTS[member.zarr_format])
-    try:
-        current = parse_stored_array(documents, member.zarr_format)
-    except RectilinearChunksDisabledError:
-        raise  # A document that can be read, but only with the flag.
-    except (KeyError, TypeError, ValueError):
-        return member
-    if current._stored_document_upgraded:
-        await upsert_metadata(store_path, current, documents)
-        object.__setattr__(current, "_stored_document_upgraded", False)
-    return current
-
-
-class _Refreshed(NamedTuple):
-    """A member of consolidated metadata that `_refresh_consolidated` read again from the
-    member's own document."""
-
-    members: dict[str, ArrayMetadata | GroupMetadata]
-    """The consolidated metadata that holds the member, by member name."""
-    name: str
-    stale: ArrayMetadata
-    current: ArrayMetadata
-
-    def adopt(self) -> None:
-        """Replace the stale member with the current one in place, so every handle that
-        shares this consolidated metadata sees it; unless the member was deleted or
-        replaced since it was read."""
-        if self.members.get(self.name) is self.stale:
-            self.members[self.name] = self.current
-
-
-async def _refresh_consolidated(
-    store_path: StorePath, metadata: GroupMetadata
-) -> tuple[GroupMetadata, list[_Refreshed]]:
-    """`metadata` with each member of its consolidated metadata that was read from a
-    document that had to be upgraded, at any depth, replaced by the metadata of the
-    member's own document as it now is (see `_refresh_array`), read concurrently, and
-    the members it replaced: the member document may have changed since, so the
-    consolidated copy is never stored as if it were valid. `metadata` is left as it is,
-    for the group to adopt the members (see `_Refreshed.adopt`) once they are stored."""
-    from zarr.core.group import GroupMetadata
-
-    consolidated = metadata.consolidated_metadata
-    if consolidated is None:
-        return metadata, []
-    groups = {
-        name: member
-        for name, member in consolidated.metadata.items()
-        if isinstance(member, GroupMetadata)
-    }
-    arrays = {
-        name: member
-        for name, member in consolidated.metadata.items()
-        if not isinstance(member, GroupMetadata) and member._stored_document_upgraded
-    }
-    nested, current = await asyncio.gather(
-        asyncio.gather(*(_refresh_consolidated(store_path / n, m) for n, m in groups.items())),
-        asyncio.gather(*(_refresh_array(store_path / n, m) for n, m in arrays.items())),
-    )
-    refreshed = [
-        _Refreshed(consolidated.metadata, name, stale, new)
-        for (name, stale), new in zip(arrays.items(), current, strict=True)
-        if new is not stale
-    ]
-    members = dict(consolidated.metadata)
-    members.update({member.name: member.current for member in refreshed})
-    for name, (group, inner) in zip(groups, nested, strict=True):
-        members[name] = group
-        refreshed.extend(inner)
-    if not refreshed:
-        return metadata, []
-    consolidated = replace(consolidated, metadata=members)
-    return replace(metadata, consolidated_metadata=consolidated), refreshed
+    return mark_upgraded(parse_array_metadata(dict(upgraded)), stored, [None] * len(readings), None)
 
 
 def _build_parents(store_path: StorePath, zarr_format: ZarrFormat) -> dict[str, GroupMetadata]:
@@ -239,9 +157,7 @@ def _build_parents(store_path: StorePath, zarr_format: ZarrFormat) -> dict[str,
 async def save_metadata(
     store_path: StorePath, metadata: ArrayMetadata | GroupMetadata, ensure_parents: bool = False
 ) -> None:
-    """Asynchronously save the array or group metadata. A group adopts, in place, the
-    members of its consolidated metadata that had to be read again to store it (see
-    `_refresh_consolidated`), once they are stored.
+    """Asynchronously save the array or group metadata.
 
     Parameters
     ----------
@@ -256,14 +172,8 @@ async def save_metadata(
     ------
     ValueError
     """
-    from zarr.core.group import GroupMetadata
-
-    refreshed: list[_Refreshed] = []
-    if isinstance(metadata, GroupMetadata):
-        # The one place group metadata is stored, and with it consolidated metadata.
-        metadata, refreshed = await _refresh_consolidated(store_path, metadata)
     to_save = metadata.to_buffer_dict(default_buffer_prototype())
-    set_awaitables = [store_documents(store_path, to_save)]
+    set_awaitables = [set_or_delete(store_path / key, value) for key, value in to_save.items()]
 
     if ensure_parents:
         # To enable zarr.create(store, path="a/b/c"), we need to create all the intermediate groups.
@@ -301,6 +211,3 @@ async def save_metadata(
             ) from e
 
     await asyncio.gather(*set_awaitables)
-    # Only once stored: a group whose store failed keeps the members flagged, to read again.
-    for member in refreshed:
-        member.adopt()

src/zarr/core/metadata/upgrades.py

@@ -18,6 +18,7 @@
 
 from __future__ import annotations
 
+import copy
 import json
 import warnings
 from collections.abc import Callable, Iterable, Mapping, Sequence
@@ -43,14 +44,17 @@
 )
 
 
-def mark_upgraded[M](metadata: M, readings: Sequence[str | None], path: str | None) -> M:
-    """Record that `metadata` was read from a stored document that needed the upgrades
-    whose `readings` `upgrade_array_document` returned, if any: set
-    `_stored_document_upgraded` on `metadata`, so the array stores the upgrade before
-    it writes chunks under it, and warn once with the readings that are warnings,
-    naming the array at `path` when the caller knows it."""
+def mark_upgraded[M](
+    metadata: M, stored: ArrayDocument, readings: Sequence[str | None], path: str | None
+) -> M:
+    """Record that `metadata` was read from the document `stored`, which needed the
+    upgrades whose `readings` `upgrade_array_document` returned, if any: keep a copy of
+    `stored` as `_stored_document` on `metadata`, so the array stores the upgrade before
+    it writes chunks under it and consolidated metadata stores it as it was stored, and
+    warn once with the readings that are warnings, naming the array at `path` when the
+    caller knows it."""
     if readings:
-        object.__setattr__(metadata, "_stored_document_upgraded", True)
+        object.__setattr__(metadata, "_stored_document", copy.deepcopy(stored))
     if messages := [reading for reading in readings if reading is not None]:
         subject = "" if path is None else f"Array {path!r}: "
         # The synchronous API parses metadata on zarr's IO thread, whose stack holds no

src/zarr/core/metadata/v2.py

@@ -25,6 +25,7 @@
         TBaseScalar,
         ZDType,
     )
+    from zarr.core.metadata.upgrades import ArrayDocument
 
 from dataclasses import dataclass, field, fields, replace
 
@@ -72,10 +73,9 @@ class ArrayV2Metadata(Metadata):
     compressor: Numcodec | None
     attributes: dict[str, JSON] = field(default_factory=dict)
     zarr_format: Literal[2] = field(init=False, default=2)
-    _stored_document_upgraded: ClassVar[bool] = False
-    """Whether `from_dict` read this metadata from a stored document it had to upgrade
-    (set on the instance by `mark_upgraded`), so the store may still hold that invalid
-    document."""
+    _stored_document: ClassVar[ArrayDocument | None] = None
+    """The stored document `from_dict` read this metadata from, if it had to upgrade it
+    (set on the instance by `mark_upgraded`): the store may still hold it."""
 
     def __init__(
         self,
@@ -209,7 +209,7 @@ def from_dict(cls, data: dict[str, Any], *, path: str | None = None) -> ArrayV2M
 
         _data = {k: v for k, v in _data.items() if k in expected}
 
-        return mark_upgraded(cls(**_data), readings, path)
+        return mark_upgraded(cls(**_data), data, readings, path)
 
     def to_dict(self) -> dict[str, JSON]:
         zarray_dict = super().to_dict()

src/zarr/core/metadata/v3.py

@@ -52,6 +52,7 @@
     from zarr.core.buffer import Buffer, BufferPrototype
     from zarr.core.chunk_grids import ChunkGrid
     from zarr.core.dtype.wrapper import TBaseDType, TBaseScalar
+    from zarr.core.metadata.upgrades import ArrayDocument
 
 
 def parse_zarr_format(data: object) -> Literal[3]:
@@ -472,10 +473,9 @@ class ArrayV3Metadata(Metadata):
     node_type: Literal["array"] = field(default="array", init=False)
     storage_transformers: tuple[dict[str, JSON], ...]
     extra_fields: dict[str, AllowedExtraField]
-    _stored_document_upgraded: ClassVar[bool] = False
-    """Whether `from_dict` read this metadata from a stored document it had to upgrade
-    (set on the instance by `mark_upgraded`), so the store may still hold that invalid
-    document."""
+    _stored_document: ClassVar[ArrayDocument | None] = None
+    """The stored document `from_dict` read this metadata from, if it had to upgrade it
+    (set on the instance by `mark_upgraded`): the store may still hold it."""
 
     def __init__(
         self,
@@ -676,7 +676,7 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Self:
             extra_fields=allowed_extra_fields,
             storage_transformers=_data_typed.get("storage_transformers", ()),  # type: ignore[arg-type]
         )
-        return mark_upgraded(metadata, readings, path)
+        return mark_upgraded(metadata, data, readings, path)
 
     def to_dict(self) -> dict[str, JSON]:
         out_dict = super().to_dict()

tests/test_array_stateful.py

@@ -172,7 +172,7 @@ def _open(self) -> zarr.Array[Any]:
         warned = any(issubclass(w.category, ZarrUserWarning) for w in record)
         must_warn = any(self.shape[axis] > 0 for axis in self.legacy_axes)
         assert warned is must_warn, [str(w.message) for w in record]
-        assert arr.metadata._stored_document_upgraded is bool(self.legacy_axes)
+        assert (arr.metadata._stored_document is not None) is bool(self.legacy_axes)
         return arr
 
     # ----------------------------------------------------------------- model