Partition-level bloom sentinels for cross-segment skipping (#47) - #35
Draft
SferaDev wants to merge 4 commits into
Draft
Partition-level bloom sentinels for cross-segment skipping (#47)#35SferaDev wants to merge 4 commits into
SferaDev wants to merge 4 commits into
Conversation
SferaDev
marked this pull request as ready for review
June 12, 2026 13:33
SferaDev
force-pushed
the
feat/partition-bloom-sentinels
branch
from
June 12, 2026 15:15
01bde7a to
4b1dbcc
Compare
SferaDev
force-pushed
the
fix/epoch-encoding
branch
from
June 12, 2026 20:11
9b59034 to
b579349
Compare
SferaDev
force-pushed
the
feat/partition-bloom-sentinels
branch
from
June 12, 2026 20:45
4b1dbcc to
99e08a9
Compare
During compression every bloom-supported (numeric/date/timestamp, non-segment-by) column accumulates a partition-wide bloom filter in a fixed 2 MiB power-of-two build buffer (PARTITION_BLOOM_HASHES = 4), folded down (OR-halving, no false negatives) to ~10 bits/element from the partition-level HLL ndistinct and stored as a _segment_id = -1 sentinel row in the existing _blooms companion table. Sentinels whose folded density exceeds 0.6 are dropped — absence of a sentinel just means no partition-level skipping (fail-safe). At query time, Phase 0pre of load_segments_heap probes the sentinel by PK before the colstats probes and the meta scan: a rejecting sentinel skips every segment in the partition for the price of one index descent per probed column. Probe constants go through bloom_probe_encode so the probe hashes the same i64 domain the build side hashed (Unix-epoch us for timestamps/dates, raw bit patterns for floats). Gated by pg_deltax.partition_bloom_filters (default on); build follows pg_deltax.bloom_filters. All three build paths write sentinels: SPI compress (compress_partition_streaming), buffered COPY backfill (flush_segment), and the threaded parquet backfill (compute_partition_bloom_hashes on workers, merged into the main thread's accumulators). Decompress and retention drop the blooms table wholesale, so stale sentinels cannot survive; BloomFilter::merge_fold (OR-merge with fold-down) is included for the future incremental-compaction path. Local ClickBench: Q19 DeltaX scan 0.96 -> 0.106 ms at 1M rows (segments=0, segments_bloom_skipped=34), 10.7 -> 5.4 ms at 10M. Extracted from the perf/clickhouse-gap-session branch (commit fcfb1dc, bloom parts only); stacks on the epoch-encoding fixes branch.
Fresh-eyes pass over the partition-bloom-sentinel diff: - bloom.rs: remove BloomFilter::merge_fold and its two tests — dead code with no production caller (sentinels are write-once per compress; the incremental-compaction PR that needs it can reintroduce it). Reword the PARTITION_BLOOM_HASHES rationale to the real current constraint. - compress.rs: extract bloom_value_hashes() as the single source of truth for the build-side value->hash mapping; compute_segment_blooms and compute_partition_bloom_hashes (parquet worker path) both use it, so the two build paths can no longer diverge. Drops the insert closure, the duplicated five-arm type matches, and the unreachable!(). - scan/exec/segments.rs: extract bloom_probe_hashes() shared by the Phase 0pre sentinel probe and the per-segment bloom checks — same op gate, type gate, and bloom_probe_encode mapping on both probe sites by construction. - PERF_IMPROVEMENTS.md: state the future-incremental-update invariant without referencing the removed function. No behavior change; net -135 lines.
The PG18 integration job failed intermittently on test_worker_drains/ retention: the worker serviced one test, then went silent for the rest of the session. Root cause (reproduced by planting a deltax_deltatable row with no backing table): any ERROR in a maintenance cycle — most commonly 'relation "<table>_default" does not exist' from the test _cleanup() race, where pg_reload_conf() wakes the worker via SIGHUP right before the catalog DELETE + DROP TABLE commit — unwound out of deltax_worker_main and terminated the process. pgrx registers bgworkers with BGW_NEVER_RESTART by default, so a single transient error silently disabled all partition maintenance until server restart. Two layers of defense: - wrap each maintenance pass in PgTryBuilder: on error, abort the open transaction (autovacuum-style sigsetjmp recovery), log, and retry on the next wakeup; - register the worker with a 5s restart time so even an unexpected death isn't permanent. Adds test_worker_survives_error_in_cycle, which plants the ghost catalog row, asserts the worker process survives the failing cycle, and verifies maintenance resumes once the bad entry is removed.
…l_idx The partition-sentinel probe directly indexed col_names/col_idx_map with bq.col_idx. On a logical-replication subscriber mid-initial-sync the partition descriptor can be empty while batch quals are present, so the probe panicked with 'index out of bounds: the len is 0 but the index is 0' (seen in CI test_scenario_2_replicate_companion_tables_directly, PG17). Use checked lookups like the per-segment bloom path (segment_filters) does; a skipped probe only forfeits pruning, never correctness.
SferaDev
force-pushed
the
feat/partition-bloom-sentinels
branch
from
June 12, 2026 21:21
99e08a9 to
0e694f5
Compare
SferaDev
marked this pull request as draft
June 12, 2026 22:41
SferaDev
added a commit
that referenced
this pull request
Jun 15, 2026
* fix: valbitmap correctness — fail-safe pruning, retention leak, cross-schema companion lookup Three pre-existing correctness bugs found during review of #31/#34/#35: 1. Text valbitmap wrong-empty-results on segment overflow. A segment with more than VALBITMAP_MAX_DISTINCT (32) distinct text values writes no valbitmap row and contributes nothing to the partition-level column_valmap. When the union of the remaining segments stayed <= 32, querying a value unique to the overflowed segment missed the valmap and took the 'prune every segment without reading the bitmap table' shortcut — returning zero rows instead of the overflowed segment's rows. Pruning now always goes through the per-segment bitmap rows: a valmap miss (empty wanted_bits) prunes exactly the segments that wrote a bitmap row; segments without one are never prunable by valmap logic. The now-redundant ValbitmapCheck::prune_all field is removed. 2. Retention drop leaked _valbitmap companions. auto_drop_partitions dropped blobs/blooms/text_lengths/colstats/meta but not valbitmap, leaving orphaned tables in _deltax_compressed forever. (decompress and the empty-partition cleanup already covered all six.) 3. check_compressed_partition matched companions by partition NAME only. Companion tables live in the single shared _deltax_compressed schema and embed only the table name, so a same-named partition in another schema (or any same-named plain table) was treated as compressed and served the other partition's data. The lookup now confirms via deltax.deltax_partition that the exact (schema, table) pair is the compressed one — at most one partition of a given name can be compressed, so is_compressed disambiguates. Regression tests: tests/test_valbitmap.py (one-segment-overflow shape), tests/test_worker.py (no orphaned companions after retention drop), tests/test_compression.py (two-schema same-name partition). * test: make overflow regression probe robust to minmax/dictionary segment skipping The plan-shape assertions (vb_skipped=1, segments=1) now ride on the present-value 'ov07' query — where only the valbitmap can skip the bitmap-covered segment and the overflowed segment must be scanned. The absent-value probe keeps only the row-count assertion: an absent constant can legitimately eliminate the overflowed segment through exact per-segment evidence (text minmax, dictionary value check) regardless of the valbitmap. * rustfmt: format worker.rs launcher fan-out (inherited unformatted from #38 merge)
wezell
pushed a commit
to dotCMS/deltax
that referenced
this pull request
Jul 14, 2026
* fix: valbitmap correctness — fail-safe pruning, retention leak, cross-schema companion lookup Three pre-existing correctness bugs found during review of xataio#31/xataio#34/xataio#35: 1. Text valbitmap wrong-empty-results on segment overflow. A segment with more than VALBITMAP_MAX_DISTINCT (32) distinct text values writes no valbitmap row and contributes nothing to the partition-level column_valmap. When the union of the remaining segments stayed <= 32, querying a value unique to the overflowed segment missed the valmap and took the 'prune every segment without reading the bitmap table' shortcut — returning zero rows instead of the overflowed segment's rows. Pruning now always goes through the per-segment bitmap rows: a valmap miss (empty wanted_bits) prunes exactly the segments that wrote a bitmap row; segments without one are never prunable by valmap logic. The now-redundant ValbitmapCheck::prune_all field is removed. 2. Retention drop leaked _valbitmap companions. auto_drop_partitions dropped blobs/blooms/text_lengths/colstats/meta but not valbitmap, leaving orphaned tables in _deltax_compressed forever. (decompress and the empty-partition cleanup already covered all six.) 3. check_compressed_partition matched companions by partition NAME only. Companion tables live in the single shared _deltax_compressed schema and embed only the table name, so a same-named partition in another schema (or any same-named plain table) was treated as compressed and served the other partition's data. The lookup now confirms via deltax.deltax_partition that the exact (schema, table) pair is the compressed one — at most one partition of a given name can be compressed, so is_compressed disambiguates. Regression tests: tests/test_valbitmap.py (one-segment-overflow shape), tests/test_worker.py (no orphaned companions after retention drop), tests/test_compression.py (two-schema same-name partition). * test: make overflow regression probe robust to minmax/dictionary segment skipping The plan-shape assertions (vb_skipped=1, segments=1) now ride on the present-value 'ov07' query — where only the valbitmap can skip the bitmap-covered segment and the overflowed segment must be scanned. The absent-value probe keeps only the row-count assertion: an absent constant can legitimately eliminate the overflowed segment through exact per-segment evidence (text minmax, dictionary value check) regardless of the valbitmap. * rustfmt: format worker.rs launcher fan-out (inherited unformatted from xataio#38 merge) (cherry picked from commit 1649c98)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Partition-level bloom sentinels (PERF #47): during compression, every bloom-supported (numeric/date/timestamp, non-segment-by) column accumulates a partition-wide bloom filter, stored as a
_segment_id = -1sentinel row in the existing<partition>_bloomscompanion table. At query time a new Phase 0pre inload_segments_heapprobes the sentinel by PK before the colstats probes and the meta scan — a rejecting sentinel skips every segment in the partition for the price of one index descent per probed column.Why
Point lookups (ClickBench Q19-class:
WHERE UserID = <const>) previously paid per-partition colstats minmax probes, a meta scan, and per-segment bloom loads even for partitions that cannot contain the value. The sentinel collapses all of that meta-hit work into a single 2-key PK probe per partition.Numbers
segments=0 segments_bloom_skipped=34,meta hit142 → 0).Design digest
(Full write-up:
dev/docs/PERF_IMPROVEMENTS.md§47, shipped in this PR.)PARTITION_BLOOM_BUILD_BYTES,PARTITION_BLOOM_HASHES = 4— fixed because the accumulator is created before the partition's final ndistinct is known), folded down (OR-halving — positions arehash % 2^n, so folding can only widen the match set, never produce false negatives) to ~10 bits/element using the partition-level HLL ndistinct at flush.pg_deltax.partition_bloom_filters(default on; build side followspg_deltax.bloom_filters). Probe constants go throughbloom_probe_encode(Fix epoch-encoding bugs in segment min/max pruning and bloom probes #33) so the probe hashes the same i64 domain the build side hashed (Unix-epoch µs for timestamps/dates, raw bit patterns for floats).compress_partition_streaming), buffered COPY backfill (flush_segment), threaded parquet backfill (workers ship value hashes viaCompressedSegment::partition_bloom_hashes; the main thread owns the accumulators).hash % 2^nmembership) or dropped — never overwritten.Testing
make build— clean.make clippy— zero warnings.make test(PG 17) — 568 unit tests passed (includes new fold/no-false-negative bloom tests).make integration-test PG_VERSIONS=17— full suite green, including the newtests/test_partition_bloom.py(sentinel creation on both compress paths, sentinel-only pruning with per-segment bloom rows deleted, GUC off behavior, IN-list probes, present-value non-pruning, decompress→recompress sentinel rebuild).Provenance
Extracted from a longer perf session (
perf/clickhouse-gap-session, commitfcfb1dc— bloom parts only; the same commit's storage-v2 segment-file code ships separately in #32, and its topn merge rework was superseded and is excluded).