Skip to content

perf(table): fold clustered deletions into range segments - #9311

Open
dshepelev15 wants to merge 1 commit into
lance-format:mainfrom
dshepelev15:up/10-range-segments
Open

dshepelev15 wants to merge 1 commit into
lance-format:mainfrom
dshepelev15:up/10-range-segments

Conversation

@dshepelev15

@dshepelev15 dshepelev15 commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

A stable-row-id table whose deletions cluster, as they do after compacting
fragments with deleted rows, ends up with RangeWithBitmap segments that
spend one bit on every row of the range. On one large table that was
890 MB of a 945 MB manifest for 7.1 billion rows whose holes form only
26 million runs.

Writing such a sequence as plain Range segments, one per run of live
rows, cuts the manifest to 323 MB and needs no format change: every
version reads Range segments. What makes many Range segments cheap to
read is the in-memory side. The decoder folds each run of consecutive,
sorted, disjoint Range segments into one U64Segment::Ranges, backed by
HoleRuns: the range starts and a prefix sum of present rows in u32, so
position and offset lookups are binary searches and the whole run costs
8 bytes per range instead of a heap-allocated enum each. The decoder also
walks the RowIdSequence wire format directly, so tens of millions of
Range segments do not each become a proto message before folding.
Encoding expands the segment back into Range segments; it is only
ever written as part of a sequence.

Re-encoding existing bitmaps is opt-in through the table config key
lance.row_ids.range_segments (set with update_config): the enabling
commit re-encodes every fragment where ranges are smaller, later commits
re-encode what they change. Older readers understand the result but,
lacking the compact form, handle thousands of segments per fragment
slowly, which is why it is not the default.

Benchmark on the manifest of that table (20,760 fragments, 9,734 bitmap
segments, 8,094 of them converted), decoded locally:

manifest file:                     945.0 MB -> 322.7 MB
inline row id bytes:               927.3 MB -> 305.0 MB
manifest decode, warm:             206-246 ms -> 148-197 ms
RSS after decode:                  995 MB -> 395 MB
sequences decode + RowIdIndex:     222-243 ms -> 248-285 ms
RSS after index:                   1041 MB -> 465 MB

Follows up on the discussion in #9272: the same saving with the existing wire format instead of a new segment type.

lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 16, 2026

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for taking the #9272 discussion the rest of the way — this is the right follow-up. Clustered deletions after compact really do make the bitmaps dominate, and writing ordinary Range segments gets the same saving without a format change. I’m good with the direction, including keeping it opt-in.

A few small cleanups, none of them blocking:

  1. Could RANGE_SEGMENTS_CONFIG_KEY rustdoc mention that turning the key off stops future conversions but does not rewrite already-converted fragments? That’s easy to miss operationally.

  2. HoleRuns is pub used today. If it’s only the payload behind U64Segment::Ranges, it might be happier as crate-private.

  3. The standalone RangesSortedArray arm isn’t on the sequence write path. I think we can drop it rather than keep a third encoding around.

@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
@dshepelev15

Copy link
Copy Markdown
Contributor Author

Thanks for the review! All three addressed in the latest push:

  1. RANGE_SEGMENTS_CONFIG_KEY rustdoc now says that setting the key back to false only stops further conversions; already-converted fragments keep Range segments until something rewrites their row ids (e.g. compaction), so reader upgrades should be coordinated before enabling it.
  2. HoleRuns is no longer re-exported from rowids, so it is not part of the public API any more. It stays pub inside the private runs module because it is the payload of the public U64Segment::Ranges variant (pub(crate) trips private_interfaces). The accessors only tests used are now #[cfg(test)].
  3. Dropped the standalone RangesSortedArray arm. Nothing produces a Ranges segment except as_ranges and the decoder fold, and from_slice / mask / slice never pick it, so the version-span path cannot reach it; the arm is now unreachable! with a message, and the test for the fallback is gone.

Commit message and description updated to match.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. and removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
@dshepelev15

Copy link
Copy Markdown
Contributor Author

Rebased on main to pick up #9099 (the two imports in manifest_build.rs were the only conflict). While at it, read_row_ids now walks the input slice directly instead of copying it into a Bytes first; that copy was the whole inline row id block per fragment and bought nothing. cargo test -p lance-table and the rowids tests in lance pass, clippy and fmt are clean.

@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 18, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 18, 2026
A stable-row-id table whose deletions cluster, as they do after compacting
fragments with deleted rows, ends up with `RangeWithBitmap` segments that
spend one bit on every row of the range. On one large table that was
890 MB of a 945 MB manifest for 7.1 billion rows whose holes form only
26 million runs.

Writing such a sequence as plain `Range` segments, one per run of live
rows, cuts the manifest to 323 MB and needs no format change: every
version reads `Range` segments. What makes many `Range` segments cheap to
read is the in-memory side. The decoder folds each run of consecutive,
sorted, disjoint `Range` segments into one `U64Segment::Ranges`, backed by
`HoleRuns`: the range starts and a prefix sum of present rows in `u32`, so
position and offset lookups are binary searches and the whole run costs
8 bytes per range instead of a heap-allocated enum each. The decoder also
walks the `RowIdSequence` wire format directly, so tens of millions of
`Range` segments do not each become a proto message before folding.
Encoding expands the segment back into `Range` segments; it is only
ever written as part of a sequence.

Re-encoding existing bitmaps is opt-in through the table config key
`lance.row_ids.range_segments` (set with `update_config`): the enabling
commit re-encodes every fragment where ranges are smaller, later commits
re-encode what they change. Older readers understand the result but,
lacking the compact form, handle thousands of segments per fragment
slowly, which is why it is not the default.

Benchmark on the manifest of that table (20,760 fragments, 9,734 bitmap
segments, 8,094 of them converted), decoded locally:

    manifest file:                     945.0 MB -> 322.7 MB
    inline row id bytes:               927.3 MB -> 305.0 MB
    manifest decode, warm:             206-246 ms -> 148-197 ms
    RSS after decode:                  995 MB -> 395 MB
    sequences decode + RowIdIndex:     222-243 ms -> 248-285 ms
    RSS after index:                   1041 MB -> 465 MB
@dshepelev15

Copy link
Copy Markdown
Contributor Author

One more pass to trim the diff (−99 lines, no behaviour change on the write path):

  • TryFrom<pb::RowIdSequence> now delegates to read_row_ids, so there is a single fold path; collapse_range_runs is gone.
  • Dropped the RangeWithHolesRanges conversion and HoleRuns::from_missing_offsets. Hole arrays are small by construction and the benchmark only ever converted bitmaps, so only RangeWithBitmap re-encodes now. The config key rustdoc says so.
  • Test-only accessors on HoleRuns replaced with direct field access from the in-file tests.

cargo test -p lance-table, the rowids tests in lance, clippy and fmt all pass.

@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 18, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Gate recommendation: approve with a non-blocking risk.

Consolidating the fold path and narrowing re-encoding to bitmap-backed segments preserve the reviewed opt-in behavior and existing wire format. The remaining operational risk is persistent old-reader slowdown: disabling lance.row_ids.range_segments stops future conversions but does not rewrite unchanged fragments, so coordinate reader upgrades before enabling it.

Please mark this PR with the breaking-change label.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 18, 2026
@lance-gatekeeper lance-gatekeeper Bot added the K-risk Latest Gatekeeper recommendation includes a non-blocking risk. label Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants