Handle a stalled store sync bridge in HDF5 translation - #588
FrancescAlted wants to merge 2 commits into
Conversation
zarr v3 memory stores hand out Buffer objects, and this helper only
converted the ones sitting at the top level of the reference dict. A
Buffer nested in a dict or list value survived, and consumers then
failed to serialize the reference set ("can not serialize 'Buffer'
object"). Walk dicts and lists too, still stripping a leading slash
from the keys of top-level buffers.
_translator catches every exception per node and, with the default error="warn", downgrades it to a warning and moves on. That is wrong for a timeout: it means the store (or zarr's sync bridge behind it) stalled, so the next call fails the same way and a caller who bounded the bridge still never gets a finished translation. Let TimeoutError through regardless of the error mode, so one bounded failure aborts the translation instead of multiplying.
| del refs[k] | ||
| refs.update(new_keys) | ||
| del refs[k] | ||
| refs[k.removeprefix("/")] = v.to_bytes() |
There was a problem hiding this comment.
This is a separate issue, right? Did you find spurious "/" characters? If so, it's probably needed in the else branch too.
There was a problem hiding this comment.
No, I have found none, and this commit doesn't change the slash handling; it was already in #516 and only looks moved because of the loop restructure.
I snapshotted the dict just before normalisation across a synthetic group/attrs/dataset hierarchy (inline_threshold 0 and huge), vlen.h5 in both vlen modes, and the hdf5_mini_*/hdf5_mali_chunk fixtures: 98 keys, 0 starting with /.
The else branch shouldn't be able to see one: everything zarr writes into that store is a Buffer (MemoryStore copies into one), so a name that leaked a leading slash would still land as a zarr-written buffer key — the branch that strips.
|
You are seeing failures due to kerchunk's incomplete transition to zarr v3, in particular the change in how to add codecs (kerchunk uses some custom codecs to deal with HDF5, particularly strings). You could test against zarr v2, or ensure that the same failures are happening in this run as were already on main. I could mark them all as xfail, as I don't think I will have time to actually fix them. |
|
I see. I was not aware that zarr v3 is not fully supported by kerchunk yet. But no worries, for the time being Blosc2 just workarounds this issue. I think it is worth keeping this here for when support for zarr v3 can make more progress; I tend to think that zarr v3 is the future, so I am not putting too much work on supporting v2. |
Absolutely, but some details remain to be ironed out. |
2 participants
We have been experiencing two failures while translating HDF5 hierarchies through zarr v3's sync bridge. This caused issues in testing python-blosc2 with kerchunk, where a translation hung CI jobs for hours (and took a wile to catch):
Make
translate_refs_serializablewalk nested values (af5eb6c)zarr v3 memory stores hand out
Bufferobjects, and the helper only converted the ones sitting at the top level of the reference dict. ABuffernested in a dict or list value survived, and consumers then failed to serialize the result (can not serialize 'Buffer' object). Dicts and lists are now walked too; keys of top-level buffers still lose a leading slash as before.Do not quash a
TimeoutErrorfrom the store sync bridge (5a9d5bd)_translatorcatches every exception per node and, with the defaulterror="warn", downgrades it to a warning and moves on. That is wrong for a timeout: it means the store (or zarr's sync bridge behind it) stalled, so the next call fails the same way, and a caller who bounded the bridge still never gets a finished translation.TimeoutErrornow passes through regardless of the error mode, so one bounded failure aborts the translation instead of multiplying.Both changes come with regression tests (
test_translate_refs_serializable_nested_buffers,test_timeout_is_not_quashed).Note on scope: zarr's sync bridge waits on its private loop with
timeout=Noneunlessasync.timeoutis set, so stock kerchunk can still wait forever if the bridge stalls. This PR makes a caller-set bound effective; the stall itself is a zarr-side issue that still lacks a minimal reproducer.