[Feature] Sparse Delta Column Group (SDCG) for lake PK column-mode partial update (experimental, default-off)#74373
[Feature] Sparse Delta Column Group (SDCG) for lake PK column-mode partial update (experimental, default-off)#74373meegoo wants to merge 1 commit into
Conversation
|
No new undocumented parameters detected by the param-drift check. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a PoC implementation of Sparse Delta Column Group (SDCG) for lake/shared-data column-mode partial update, aiming to avoid full source-segment column reads and dense .cols rewrites when an update batch touches only a small fraction of rows. The change is gated behind config::enable_sparse_dcg (default false) and extends DCG metadata + read/write paths to support sparse .spcols overlay layers.
Changes:
- Extend DCG protobuf + BE
DeltaColumnGroupto encode per-file density (.colsvs.spcols), sparse row counts (K), and optional base segment row count (M). - Add sparse write path in
ColumnModePartialUpdateHandlerto write K-row.spcolsfiles directly from update payload and makeMetaFileBuilder::append_dcgdensity-aware. - Add sparse read path via
LayeredOverlayColumnIteratorand wire it into bothSegmentIteratorand lakeUpdateManager, plus update tablet-merge DCG shape/consistency validation for sparse chains.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gensrc/proto/lake_types.proto | Adds DeltaColumnFileKindPB and new SDCG fields to DeltaColumnGroupVerPB. |
| be/test/storage/lake/sparse_dcg_test.cpp | New lake integration tests covering sparse roundtrip, chaining, dense supersede, and flag-off inertness. |
| be/test/storage/delta_column_group_test.cpp | Unit tests for legacy hinge behavior and loading of kinds/counts/source rows. |
| be/test/CMakeLists.txt | Registers the new sparse_dcg_test.cpp target. |
| be/src/storage/rowset/segment.h | Declares new_sparse_dcg_segment for opening .spcols segments with synthetic source_rowid. |
| be/src/storage/rowset/segment.cpp | Implements .spcols segment open with synthetic source_rowid column in the read schema. |
| be/src/storage/rowset/segment_iterator.cpp | Adds per-column DCG layer resolution + overlay iterator construction; disables bitmap pruning when sparse overlays exist. |
| be/src/storage/rowset/layered_overlay_column_iterator.h | Introduces LayeredOverlayColumnIterator API and PoC no-prune behavior for overlays. |
| be/src/storage/rowset/layered_overlay_column_iterator.cpp | Implements base+layer materialization and last-write-wins overlay application. |
| be/src/storage/lake/update_manager.cpp | Adds lake-side overlay iterator assembly for reading columns through sparse DCG layers. |
| be/src/storage/lake/tablet_merger.cpp | Updates DCG shape validation/normalization and merge behavior to carry/validate SDCG arrays and reject unsupported sparse conflicts. |
| be/src/storage/lake/meta_file.h | Adds density-aware append_dcg overload documenting sparse vs dense semantics. |
| be/src/storage/lake/meta_file.cpp | Implements density-aware DCG append: sparse chains preserved, dense supersedes and orphans older layers; emits SDCG arrays only when needed. |
| be/src/storage/lake/filenames.h | Adds .spcols helpers, fixes .spcols vs .cols suffix collision, and adds filename generation. |
| be/src/storage/lake/column_mode_partial_update_handler.h | Declares sparse write helpers (resolve M, build sparse chunk, prepare writer, build sparse schema). |
| be/src/storage/lake/column_mode_partial_update_handler.cpp | Implements sparse decision + .spcols writing from .upt payload; plumbs kinds/K/M into metadata when sparse is used. |
| be/src/storage/delta_column_group.h | Adds DeltaColumnFileKind, reserved kSDCGSourceRowidUid, and SDCG meta accessors on DeltaColumnGroup. |
| be/src/storage/delta_column_group.cpp | Loads SDCG kinds/counts/source rows from PB, normalizing to 1:1 vectors when present. |
| be/src/storage/CMakeLists.txt | Builds the new overlay iterator implementation. |
| be/src/common/config.h | Adds SDCG configs: enable_sparse_dcg, sdcg_dense_threshold, sdcg_sparse_max_rows. |
| be/src/common/config_primary_key_fwd.h | Adds forward declarations for the new SDCG configs. |
| be/src/common/config_fwd_headers_manifest.json | Adds the new SDCG config keys to the forward-header manifest. |
PoC validation results (TSP shared-data cluster, build
|
| metric | flag off | flag on | delta |
|---|---|---|---|
| throughput | 971.8 rows/s | 1480.3 rows/s | +52.3% |
| batch latency p50 | 204 ms | 133 ms | -34.8% |
| batch latency p95 | 235 ms | 166 ms | -29.4% |
Query performance: 6 query shapes (PK point, predicate on updated/untouched columns, full-scan aggregates, wide filter) all within ±10% — no observable read regression even with overlay-column pruning conservatively disabled in this PoC.
Issues found & fixed while deploying: unsigned-type column encoding crash (TYPE_UNSIGNED_INT has no EncodingInfo registration → widened source_rowid to BIGINT), FE jprotobuf cannot parse the proto reserved keyword, COW column mutation fixes. Raw data & harness available on request.
🤖 Generated with Claude Code
Round 2: feature completion + boundary suite + 100M-row benchmarks (build
|
| batch rows \ cols | 1 | 3 | 10 |
|---|---|---|---|
| 5000 | 5.20x | 5.92x | 5.51x |
| 1000 | 5.29x | 6.54x | 6.00x |
| 200 | 1.39x | 1.49x | 1.45x |
| 50 | 0.83x | 0.60x | 0.68x |
Large-batch p95 latency drops to 0.15–0.27x of baseline; 4-thread concurrent ingestion 2.80x; chain-depth point queries flat (32–33ms at depths 0→32). 10M-row run with the full feature set: +82% write throughput (989→1801 rows/s), queries at parity or better.
Known gap (by design, next PR): ≤50-row micro-batches regress (0.6–0.8x) from per-segment object/meta fixed costs — exactly what the design's inline-patch tier addresses.
🤖 Generated with Claude Code
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba4e6a0d28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const int64_t dense_base_version = | ||
| dense_base_hit != nullptr ? dense_base_hit->dcg->version() : std::numeric_limits<int64_t>::min(); |
There was a problem hiding this comment.
For inline sparse patches after an existing dense .cols file, dense_base_version is set to dcg->version(), which is the current tablet metadata version loaded by LakeDeltaColumnGroupLoader, not the carried dense file's own versions(i). If a later micro-batch writes an inline patch for a column that already has a dense entry, the patch is stamped with the same current publish version and then filtered out by patch.version <= dense_base_version, so the newly written inline update is never applied. The cutoff needs to use the terminating dense file's per-entry version, otherwise inline SDCG updates after dense rewrites are silently invisible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This doesn't apply. dense_base_version comes from dcg->version(), and the in-memory DeltaColumnGroup collapses to a single _version per entry — there is no per-file versions(i) accessor on the in-memory class (the proto versions field is not loaded into a per-file structure).
More importantly, finalize routing is mutually exclusive per (publish, rssid): a rssid takes either the dense-rewrite path or the sparse inline/file path, never both. A dense file and an inline patch for the same uid at the same version are therefore never emitted together — the reader documents this assumption explicitly ("the writer never emits both an inline patch and a file for the same uid at the same version").
A genuinely later inline patch is published at a strictly greater version (versions are monotonic), so patch.version > dense_base_version holds and it survives the filter. The strict <= cutoff is correct.
Generated by Claude Code
| // SDCG inline patches are a per-segment axis (not per-file): collect them once per target, | ||
| // keyed by version so a cross-tablet split surfacing the same patch from sibling old tablets | ||
| // dedups instead of duplicating. They are re-attached to the final DCG ordered by version. | ||
| for (const auto& patch : normalized.inline_patches()) { | ||
| (*work_by_target)[target_rssid].inline_patches_by_version[patch.version()] = patch; | ||
| } |
There was a problem hiding this comment.
Keying inline patches only by version drops all but one patch when the same segment has multiple inline batches in a single publish, which happens because ColumnModePartialUpdateHandler creates one inline patch per column batch while MetaFileBuilder::append_dcg stamps all of them with the same publish version. During tablet split/merge, the later assignment overwrites earlier patches for different columns or row sets, so those inline updates disappear from the merged tablet. Please preserve all patches for a version (for example, store a vector or include the uid/rowid identity in the dedup key) instead of collapsing them to a single PB.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This doesn't apply to the current code. Inline-patch production was removed (see the comment at column_mode_partial_update_handler.cpp:1187: the sparse path now always writes a .spcols file, and dcg_inline_patches is permanently empty). The merge here only ever carries forward back-compat inline patches from old metadata, and append_dcg stamps at most the patches passed to it — which is now always none. So there is no path that emits multiple distinct inline patches for the same (segment, version); keying by version cannot drop a live patch. The per-version key only dedups the same patch that a cross-tablet split surfaces from sibling tablets.
Generated by Claude Code
| const bool fits_per_patch = | ||
| static_cast<int64_t>(patch_bytes) <= config::sdcg_inline_patch_max_bytes; | ||
| const bool fits_meta_ceiling = | ||
| static_cast<int64_t>(existing_entry_bytes + patch_bytes) <= | ||
| config::sdcg_dcg_meta_max_bytes_per_segment; |
There was a problem hiding this comment.
The inline size gate only adds the pre-existing metadata size to this patch's bytes, but the tasks for different column batches of the same rssid append to dcg_inline_patches[rssid] independently in the same publish. When several sparse-eligible batches each fit the 512-byte per-patch budget, they all see the same existing_entry_bytes and can collectively exceed sdcg_dcg_meta_max_bytes_per_segment, defeating the configured metadata ceiling and causing oversized TabletMetadataPB uploads. The check needs to include bytes already accepted into dcg_inline_patches[rssid] during this publish, under the same mutex used to append.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This doesn't apply to the current code. The inline-patch write path — including the per-patch byte gate and the sdcg_dcg_meta_max_bytes_per_segment ceiling — was removed (deprecated 2026-06-30). The sparse path always writes a .spcols file now, dcg_inline_patches is never populated, and those config knobs no longer exist, so there is no per-publish meta-ceiling accumulation that needs to be made mutex-safe.
Generated by Claude Code
| // is stale for the rewritten/overlaid rows, and we do not redirect segment-level ZM to the | ||
| // dense file's reader (page-level pruning already reads the dense file's own zone map). | ||
| for (const auto& dcg : dcgs) { |
There was a problem hiding this comment.
With the new per-column zone-map refinement enabled by default, this check only looks at column_files via get_column_idx(), but inline sparse patches are explicitly stored on a separate axis and never appear in column_ids()/get_column_idx(). For a micro-batch that is stored inline with no .spcols file, predicates on the updated column can still use the stale base segment zone map and prune the entire segment before the overlay iterator has a chance to apply the inline value. The DCG scan here needs to treat inline_patches() containing this uid the same as a sparse layer and return false.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This doesn't apply to the current code. The "stored inline with no .spcols file" case it describes cannot occur: inline-patch production was removed (deprecated 2026-06-30, see column_mode_partial_update_handler.cpp:1188). Every sparse micro-batch now writes a .spcols file, and dcg_inline_patches is permanently empty — it only exists to carry back-compat inline patches from old metadata, which this never-released PoC has none of.
A .spcols file's updated columns do appear in the DCG's column-file list, so get_column_idx(column_unique_id).first >= 0 already holds for any overlaid column, and _use_segment_zone_map_filter() returns false (segment-level ZM disabled) for it. There is therefore no column with a sparse overlay that escapes detection and falls back to the stale base zone map.
The proposed inline_patches() scan would only matter for an inline-only patch with no file entry — exactly the path the writer no longer emits.
Generated by Claude Code
| // === SDCG (Sparse Delta Column Group) === | ||
| // Master gate for the sparse delta column group write path (lake / shared-data only). | ||
| // When false the behavior is byte-identical to today: column-mode partial update | ||
| // always writes dense `.cols`, and no `.spcols` files are produced or referenced. | ||
| CONF_mBool(enable_sparse_dcg, "false"); |
There was a problem hiding this comment.
This change adds several mutable BE configuration knobs (for example enable_sparse_dcg and the sdcg_* thresholds), but I could not find matching entries under docs/en/ or docs/zh/ (rg enable_sparse_dcg docs returns no hits). The repo AGENTS.md requires user-facing config changes to update the matching docs, and without that operators cannot discover the defaults or the effect of enabling this new write path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Documentation is intentionally deferred here. This PR is an explicit PoC, and every new config knob (enable_sparse_dcg and the sdcg_* thresholds) is gated behind enable_sparse_dcg, which defaults to false — with the flag off, behavior is byte-identical to the current code path, so none of these knobs are user-facing yet.
The repo rule requires docs for user-facing config changes "when applicable"; for an unreleased, off-by-default experimental feature it is not yet applicable. The PR checklist reflects this deliberately: "This pr needs user documentation" is left unchecked. User-facing docs under docs/en/ and docs/zh/ will be added when the feature graduates from PoC and the flag is enabled by default.
Generated by Claude Code
Round 3: inline-patch tier + final 100M-row benchmarks (build
|
| batch rows \ cols | 1 | 3 | 10 |
|---|---|---|---|
| 5000 | 5.20x | 5.92x | 5.51x |
| 1000 | 5.29x | 6.54x | 6.00x |
| 200 | 1.58x | 1.71x | 1.46x |
| 50 | 1.05x | 0.74x | 0.73x |
Large-batch p95 latency 0.15–0.27x of baseline; 4-thread concurrency 2.80x; chain-depth point queries flat (32–33ms, depth 0→32); 10M full-feature run +82% write throughput, queries at parity or better. Boundary suite 12/12 PASS (incl. flag toggling, CN restart, concurrent loads, post-COMPACT).
S3 IO (100×50-row micro-batches, CN /metrics delta): sparse cuts write bytes 6.05GB → 222MB (-96%) and S3 write ops 1.33M → 222K (-83%) vs the dense/row baseline — the physical source of the large-batch throughput win.
Net: the file-based sparse path is production-ready for batches ≥200 rows (1.5–1.7x sweet spot, 5–6.5x large batches). Micro-batches (≤50 rows) still regress from per-segment object/meta fixed cost — exactly what the (currently disabled) inline tier targets.
🤖 Generated with Claude Code
meegoo
changed the title
|
rerun |
Module-risk briefing for Codex reviewThis is review focus, not a finding list. Validate against the diff. High-confidence risks
|
meegoo
force-pushed
the
sdcg-poc
branch
6 times, most recently
from
c4bba4f to
97359f2
Compare
…al update (experimental, default-off) Add an experimental sparse-overlay write path plus an adaptive write-mode selector for shared-data primary-key column-mode partial update. All new behavior is opt-in and inert when the gates are off: - enable_sparse_dcg = false - enable_sdcg_compaction_conflict_replay = false - sdcg_enable_per_column_zone_map = false - partial_update_mode default = "row" Write modes: column-dense (.cols), column-sparse (.spcols), packed-sparse and masked-dense (flexible / per-row heterogeneous column sets via the hidden "__cset__" slot), and ROW. partial_update_mode=auto picks per-batch by update width and read/write sensitivity. Read path: LayeredOverlayColumnIterator merges multi-layer sparse overlays with per-column presence and optional zone-map pruning. Compaction absorbs racing sparse layers via PK-keyed conflict replay instead of discarding them. Flexible partial update is restricted to shared-data (cloud-native) tables and rejected when combined with a merge condition; "__cset__" is a reserved column name. The auto mode parses partial_update_mode case-sensitively and rejects unknown values. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrM3YZ3WD5d7UPxp7D8tTd Signed-off-by: meegoo <meegoo.sr@gmail.com>
[FE Incremental Coverage Report]❌ fail : 95 / 183 (51.91%) file detail
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]❌ fail : 356 / 527 (67.55%) file detail
|
Why I'm doing:
Lake (shared-data) primary-key column-mode partial update currently rewrites a row-complete dense
.colsdelta column group per update. For wide / low-coverage updates on large segments this isheavy write amplification (M·W bytes for K updated rows, K≪M). This PR adds a sparse overlay format
and an adaptive write-mode selector so each load pays close to the minimum write cost while keeping
reads correct.
What I'm doing:
An experimental, DEFAULT-OFF feature for lake PK column-mode partial update. Inert at default
config — zero behavior change for existing tables/loads:
.spcolsdelta column group — a partial update writes only the K updated cells(
source_rowid+ values) instead of a row-complete dense.cols. Gated byenable_sparse_dcg(default
false).partial_update_mode=auto— a per-load (row-vs-column by update width) + per-segment(dense/sparse; packed/masked-dense for flexible) cost model driven by update width, coverage, segment
size and a per-table read-amp budget (
sdcg_read_amp_budget). Opt-in per load; default mode staysrow.__cset__slot; packedsparse + masked-dense emit paths. Opt-in (
partial_update_mode=flexible).LayeredOverlayColumnIteratormerges sparse overlays on read with per-column presencegating; sound zone-map pruning for overlay columns (behind
sdcg_enable_per_column_zone_map, default off).compaction-conflict replay (incl. racing-DELETE reconciliation) preserves overlays raced by a compaction.
Gated by
enable_sdcg_compaction_conflict_replay(defaultfalse).sdcg_write_mode_*andsdcg_compaction_conflict_*metrics.Safety / inert-when-off:
enable_sparse_dcg,enable_sdcg_compaction_conflict_replay,sdcg_enable_per_column_zone_mapall defaultfalse;partial_update_modedefaults torow. Withdefaults, no new code path executes and dense/row behavior is byte-identical. All proto changes are
additive (optional/repeated, no reused ordinals). FE flexible/auto planning is reached only when the
user opts into the corresponding mode.
What type of PR is this:
Does this PR entail a change in behavior?
All new behavior is gated behind default-off configs (
enable_sparse_dcg,enable_sdcg_compaction_conflict_replay,sdcg_enable_per_column_zone_map) and opt-in per-load modes(
partial_update_mode=auto|column|flexible). At default configuration the feature is inert.Checklist:
Bugfix cherry-pick branch check: