Skip to content

fix(encoding): handle u16 num_levels overflow in miniblock List encoding - #6989

Merged
Xuanwo merged 3 commits into
lance-format:mainfrom
touch-of-grey:VariableList2.2Fix
May 29, 2026
Merged

fix(encoding): handle u16 num_levels overflow in miniblock List encoding#6989
Xuanwo merged 3 commits into
lance-format:mainfrom
touch-of-grey:VariableList2.2Fix

Conversation

@touch-of-grey

@touch-of-grey touch-of-grey commented May 29, 2026

Copy link
Copy Markdown
Contributor

Surfaced while benchmarking mem_wal HNSW flush at scale (#6901 made flush
rebuild the persisted secondary indexes; the in-memory graph's
__neighbors/__dists columns are List<UInt32>/List<Float32>).
Flushing a generation larger than ~32k nodes at v2.1 panicked the
lance-cpu thread with chunk_bytes <= max_chunk_size; writing those
index files at v2.2 cleared the write panic but produced a corrupt read,
panicking the struct decoder with
Mismatch in length (at offset=83300) expected 100 got 92 (counts vary
run-to-run with HNSW non-determinism). HNSW::schema() is shared with
every IVF_HNSW index, so this is a general codec correctness bug, not
specific to mem_wal.

Root cause: the miniblock structural codec stores each chunk's
num_levels as u16 in the v2.2 on-disk header
(encodings/logical/primitive.rs:467). The HNSW persistence shape — a
dense level-0 prefix followed by ~6× as many empty higher-level rows —
has a healthy global levels/values ratio (~1.19), so
repdef_too_sparse_for_miniblock fell through to miniblock. The final
value chunk via slice_rest then absorbed all trailing empties (242 048
levels for a 40k-dense + 240k-empty case), num_chunk_levels as u16
silently truncated to 45 440, and the decoder short-read.

v2.2 is a stable on-disk format (V2_2 < Next), so the per-chunk header
cannot widen. Fix is code-only:

  • Heuristic. Extend repdef_too_sparse_for_miniblock with
    any_chunk_levels_overflow_u16, a single linear pass over the def
    levels that mirrors the encoder's chunking (MAX_MINIBLOCK_VALUES
    visible values per non-last chunk; slice_rest for the final) and
    returns true if any chunk would carry more than u16::MAX levels.
    Routes affected shapes to fullzip through the existing dispatcher.
    Early-exits at max_visible_level = None and def_levels.len() <= u16::MAX keep dense / small-page paths zero-cost (encoder bench delta
    within criterion noise).
  • Codec safety net. as u16 becomes
    u16::try_from(...).map_err(Error::invalid_input_source)? so any
    future shape that slips past the heuristic surfaces a clear error
    rather than corrupting data.

A log::warn! records when the heuristic overrides an explicit
STRUCTURAL_ENCODING_MINIBLOCK request. New
test_list_hnsw_shape_auto_routes_around_miniblock_overflow_v2_2
reproduces the Mismatch in length panic on HEAD~1 and round-trips
cleanly on this commit; existing test_sparse_large_string_list
parametrization still passes both encodings.

Follow-ups (not in this PR): (a) a real value chunk straddling the
simulation's 4096-value boundary can split an empty cluster across two
sim chunks; the codec safety net catches that as a clean error rather
than corruption — tightening to a sliding window is follow-up. (b)
List<Dictionary<...>> on the too-sparse path hits a pre-existing
unreachable! in encode_full_zip; not introduced here but the
heuristic broadens the surface.

The miniblock structural codec stores each chunk's num_levels as a u16 in
the v2.2 on-disk header. For shapes where a single chunk packs more than
65 535 levels (e.g. the HNSW persistence pattern: a dense level-0 prefix
followed by ~6x as many mostly-empty higher-level rows, where the final
chunk via slice_rest absorbs every trailing empty), num_chunk_levels as
u16 silently truncated and the decoder short-read the chunk, corrupting
the round trip (struct.rs:382 length-mismatch panic on read).

repdef_too_sparse_for_miniblock previously only looked at the global
levels/values ratio, which is healthy for this shape (~1.19) so the
heuristic fell through to miniblock and the corruption fired. Extend the
heuristic with any_chunk_levels_overflow_u16, a single linear pass that
simulates the encoder's chunking (MAX_MINIBLOCK_VALUES visible values per
non-last chunk; final chunk via slice_rest absorbs trailing empties) and
returns true if any chunk would carry more than u16::MAX levels.

Early-exit when the rep/def levels are absent (no list nesting) or when
the total def-level count itself fits in u16, so dense / non-list paths
pay zero added cost (encoder bench delta vs baseline: <0.5%, within noise).

As a codec safety net, replace the silent num_chunk_levels as u16 cast at
the chunk-build site with a u16::try_from that errors with a clear
'fullzip required' message — dormant in normal operation now that the
heuristic catches the shape upstream, but prevents future shapes from
silently corrupting data if they sneak past the heuristic.

v2.2 is a stable on-disk format (is_unstable() = self >= Next, and
V2_2 < Next), so widening the header isn't an option; the proper fix at
the codec layer is to route affected shapes away from miniblock.
…rride

Address cosmetic review items from the round-0 cross reviews:

- Correct '5x' doc comment to '~6x' (matches the 240k/40k test ratio).
- Rename local 'chunk_val_size' to 'max_visibles_per_chunk' for clarity.
- Use Error::invalid_input_source instead of Error::internal at the
  chunk-levels try_from site — this is a user-shape error, not an
  internal invariant violation.
- Emit a log::warn when the user explicitly requested
  STRUCTURAL_ENCODING_MINIBLOCK but the heuristic vetoed it and routed
  to fullzip, so the override is observable rather than silent.

No behavioural change for already-supported shapes; only metadata/log
output and the error variant differ.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the bug Something isn't working label May 29, 2026
@touch-of-grey touch-of-grey changed the title fix(encoding): detect per-chunk num_levels u16 overflow in miniblock List encoding fix(encoding): handle u16 num_levels overflow in miniblock List encoding May 29, 2026
The plural 'visibles' is flagged by crate-ci/typos as a misspelling of
'visible'. Use the singular adjective form for the local variable names
in any_chunk_levels_overflow_u16.
@codecov

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.42424% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../lance-encoding/src/encodings/logical/primitive.rs 90.38% 4 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@Xuanwo
Xuanwo merged commit 2eb4b9f into lance-format:main May 29, 2026
28 checks passed
Xuanwo added a commit that referenced this pull request May 29, 2026
Resolve conflicts with #6989: drop the competing repdef_too_sparse_for_miniblock / any_chunk_levels_overflow_u16 heuristic in favor of this PR's structural page splitting, which keeps the dense prefix on mini-block pages instead of falling back to fullzip. Port #6989's HNSW regression tests with assertions updated to expect the split mini-block layout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants