Repository navigation
Conversation
antiguru
marked this pull request as ready for review
October 12, 2026 09:26
antiguru
added a commit
that referenced
this pull request
Oct 15, 2026
*Posted by Claude Code on behalf of @antiguru.* Temporal bucket chunks were sorted by data, so every chunk spanned the bucket's times. Splitting a bucket at a time therefore loaded and rewrote every chunk, spilled ones included, and the bucket at the frontier splits several times per sweep. This stores updates as `((time, data), time, diff)` in the existing merge batcher. Each chunk then covers a narrow range of times, and `seal` at the midpoint passes every chunk whose time bounds don't straddle it through without loading it. The trailing time stays the batcher's time, so `seal` works unchanged. Revealed chunks convert back to `(data, time, diff)` columns one at a time. The cost is a second time column per stored record. In a local measurement with 2M records over a 2^20-tick window, swept in 2^12-tick steps with the spill pool enabled, the slowest peel-and-restore step dropped from 65 ms to 8 ms and the whole sweep from 590 ms to 384 ms. New unit tests `peel_matches_reference_{resident,spilled}` compare bucket chain peels against a reference accumulation, with retractions, colliding updates, and updates arriving for buckets that earlier peels split. Independent of #39756, which commits the readied side of these seals. The two conflict textually at the batcher field. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
Splitting a temporal bucket seals its merge batcher at the midpoint, and `AccountedChunkMerger::extract` grades the readied side resident because arrange sites load it right away. The bucket chain instead holds that side, the lower half of the bucket, in the new lower bucket, and a peel holds whole revealed buckets before the operator emits them. With spill enabled, the whole lower half of a large bucket therefore sat on the heap at once while the pool stayed flat. Add a `COMMIT_READIED` const parameter to `AccountedChunkMerger` and a `CommittingChunkBatcher` alias that commits the readied side like the kept side. The bucket wrapper uses it. Arrange sites keep the resident readied side. Co-Authored-By: Claude Opus 5.5 <[email protected]>
antiguru
force-pushed
the
moritz/temporal-bucket-commit-readied
branch
from
October 15, 2026 16:29
46e1673 to
4740c0a
Compare
Member
Author
|
Posted by Claude Code on behalf of @antiguru. Closing as no longer needed. Since #39760, bucket chunks are time-major, so a split moves every chunk that does not straddle the midpoint without loading it, and spilled chunks keep their spill state. Committing the readied side would only affect the few straddling chunks, and on a peel it adds a spill-and-reload round trip for data the operator emits right away. |
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
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.
Posted by Claude Code on behalf of @antiguru.
Splitting a temporal bucket seals its merge batcher at the midpoint.
AccountedChunkMerger::extractgrades the readied side resident, because arrange sites load it right away. The bucket chain instead keeps that side, the lower half of the bucket, in a new lower bucket, and a peel holds whole revealed buckets before the operator emits them. With spill enabled, a split therefore put the lower half of a large bucket on the heap at once while the pool stayed flat, which OOM-killed an 8 GiB replica in a reproduction with a quarter of TPC-H SF100lineitembehindmz_now()bounds.This adds a
COMMIT_READIEDconst parameter toAccountedChunkMergerand aCommittingChunkBatcheralias that commits the readied side like the kept side. The temporal bucket wrapper uses it, and arrange sites keep the resident readied side. A new unit test,committing_seal_readies_spilled_chunks, checks that a committing seal spills readied chunks past the spill floor.🤖 Generated with Claude Code