Skip to content

Profile JOIN state write granularity and retain the measured faster layout - #254

Merged
jordepic merged 2 commits into
mainfrom
perf/join-record-state
Sep 26, 2026
Merged

jordepic merged 2 commits into
mainfrom
perf/join-record-state

Conversation

@liuyongvs

Copy link
Copy Markdown
Collaborator

Sparse JOIN updates currently rewrite a complete RocksDB bucket. This investigation confirms that cost but rejects a storage-only substitution: per-record entries with whole-bucket hydration reduce writes while making the measured workloads slower. The production backend and checkpoint formats remain unchanged.

The PR adds an ignored release-build diagnostic and documents the decision. Both layouts use the same RocksDB options, binary keys, payloads, metadata and bundle lifetime. The prototype optimistically knows the changed record and omits migration/journaling overhead. It is test-only and cannot read production checkpoints.

Shape Bucket median Record median Record / bucket
Unique key 0.001733 s 0.003833 s 2.21x
Uniform small bucket 0.001628 s 0.004429 s 2.72x
Hot key, 4,096 rows 0.269512 s 0.655145 s 2.43x

For 64 hot-key bundles, logical writes drop from 274,728,512 to 68,416 bytes. Logical reads remain about 262-267 MiB and hydrated payload remains 4 MiB. SST counters and their cache/async-compaction limitations are documented. These are state microbenchmarks without SQL/source/sink costs, not whole-job performance claims.

Validation: 549 Rust tests pass in release+mimalloc, one opt-in diagnostic ignored; the diagnostic ran with two warmups and five trials per layout/shape; cargo fmt --all -- --check passes.

Refs #244. Keep that issue open for input-side point probes, lazy opposite-side iteration and the required TTL/retraction/checkpoint/rescale compatibility work. This PR records evidence and does not claim to solve JOIN write amplification.

…ayout

Sparse changes to a large JOIN bucket rewrite unchanged rows. A release
state probe confirms the write amplification, but substituting per-record
entries while retaining full-bucket hydration makes unique, uniform and hot
key workloads 2.21x, 2.72x and 2.43x slower respectively. Hot-key logical
writes fall from 262 MiB to 67 KiB per 64 bundles; iterator and hydration
costs outweigh that saving in the measured foreground loop.

Keep the existing production format and retain an opt-in diagnostic with
logical bytes, RocksDB SST counters and serialization timings. Document
counter limits and why a useful redesign must also change point probes and
opposite-side iteration. No checkpoint migration or new backend is shipped.

Validation: 549 release Rust tests pass, one diagnostic ignored by default;
release+mimalloc profile: two warmups and five trials for each shape/layout.
These are state microbenchmarks, not end-to-end SQL performance results.

Refs #244; access-pattern redesign and write amplification remain open.

@jordepic jordepic 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.

Reviewed the isolated, ignored RocksDB diagnostic and the decision documentation. The prototype remains test-only, preserves the production format, and explicitly distinguishes reduced logical writes from slower whole-bucket hydration. The benchmark limitations and remaining recovery/access-pattern work are recorded, and #244 should remain open. All 47 PR checks passed. No blocking code findings. The regular-join documentation overlaps #251; I will reconcile those paragraphs during integration. Validation here was source/test review plus completed CI, without rerunning the release diagnostic.

@jordepic

Copy link
Copy Markdown
Collaborator

Resolved the overlap with merged #251 in 7196aea by retaining both the bounded-candidate description and the whole-bucket storage findings. The diagnostic source is byte-for-byte unchanged from the reviewed PR head. The final diff against current main is the original five-file diagnostic/documentation change, and git diff origin/main --check passes. The merge-parent-wide whitespace check also encountered pre-existing CRLF CSV files imported from main; those files are unchanged relative to main.

The original PR head had all 47 checks passing. The updated head is now awaiting its required checks; enabling automatic merge after those pass. #244 remains open for the separately documented access-pattern redesign.

@jordepic

Copy link
Copy Markdown
Collaborator

GitHub rejected enabling automatic merge because this repository does not allow auto-merge. Leaving this approved PR open while the new required checks run; the documentation conflict is resolved.

@jordepic jordepic 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.

Re-reviewed the integrated head: the conflict resolution preserves both documentation changes, the diagnostic source is unchanged from the previously reviewed/tested head, and the diff against main passes the whitespace check. Approved, with merge awaiting the required CI checks. Auto-merge is unavailable in this repository.

@jordepic
jordepic merged commit c894667 into main Sep 26, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants