etl: replay sorted non-overlapping runs sequentially instead of heap-merging - #22846
etl: replay sorted non-overlapping runs sequentially instead of heap-merging#22846AskAlexSharov wants to merge 10 commits into
Conversation
…ap-merging sortableBuffer now tracks input sortedness in Put; the collector records first/last key per flushed run. When every run is sorted and consecutive runs do not overlap, Load replays providers in order instead of building the k-way merge heap. Equal keys at run boundaries keep run order, which is exactly the heap's tie-break, so the output stream is byte-identical - including duplicate keys under DupSort tables. Producers with sorted keys (ii indexKeys, caplin antiquary collectors) benefit with no call-site changes. Load of a monotone-key multi-run workload: -54% (100k/500k keys x4 vals, Apple M4 Max); sorted single-value runs -38..43%; unsorted (heap) path unchanged; Put pays ~2-3% for the sortedness compare in the pure-Put microbenchmark. Claude-Session: https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91
There was a problem hiding this comment.
Pull request overview
This PR optimizes db/etl load-time merging by adding a fast path for the common case where spilled runs are already globally ordered and non-overlapping, allowing sequential replay instead of k-way heap merging (targeting high-volume ETL producers like the inverted index key stream and Caplin antiquary collectors).
Changes:
- Track whether keys arrived in non-decreasing order inside
sortableBuffer.Put, and record per-flush run boundaries (first/last) when that condition holds. - Refactor
mergeSortFilesto choose between sequential replay (when runs are sorted and non-overlapping) and the existing heap merge, sharing the same per-element processing logic. - Add equivalence/unit tests and a benchmark to validate correctness and quantify the performance impact.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| db/etl/collector.go | Records flushed-run metadata and adds sequential replay path in mergeSortFiles (fallback to heap when needed). |
| db/etl/buffers.go | Adds input-sortedness tracking to sortableBuffer during Put. |
| db/etl/etl_test.go | Updates tests for the mergeSortFiles signature change (runs metadata now passed separately). |
| db/etl/sorted_replay_test.go | New tests covering run decision logic, stream equivalence vs heap merge, DupSort behavior, and fallback cases. |
| db/etl/sorted_bench_test.go | New benchmark simulating monotone dup-key workload similar to inverted index collection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Review follow-up: removed the per- |
Every run is sorted after Sort, so sequential-replay eligibility needs only non-overlapping boundaries, captured post-Sort inside the flush. This deletes the per-Put key compare from the hot path and lets the fast path also serve producers whose runs happen not to overlap. Claude-Session: https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91
A recorded run is never empty; first-read EOF means a truncated spill. Fail loudly like the heap path instead of silently dropping the run. Claude-Session: https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
db/etl/collector.go:401
- The two load drivers handle 'first read of a run failed' differently:
replaySequentiallyreturns an error, whilemergeViaHeappanics. That means a truncated/corrupt spill can crash the process on the heap path but only error on the sequential path. Consider unifying behavior (preferably return an error in both paths, since this is a library/data-layer utility) so failure semantics are consistent regardless of which driver is selected.
func replaySequentially(logPrefix string, providers []dataProvider, process func(key, value []byte) error) error {
for i, provider := range providers {
// A recorded run is never empty, so EOF on the first read means a
// truncated or corrupt spill; fail loudly like the heap path does.
key, value, err := provider.Next()
if err != nil {
return fmt.Errorf("%s: reading first element of run %d of %d (provider=%s): %w",
logPrefix, i, len(providers), provider, err)
}
for {
db/etl/collector.go:410
- This error message is reused for all
dataProviderimplementations, including in-memory providers, so 'from disk' can be misleading. Consider changing it to 'from provider' (or include the provider via%s) to make debugging accurate in both RAM and file-backed cases.
return fmt.Errorf("%s: error while reading next element from disk: %w", logPrefix, err)
db/etl/collector.go:73
- The docs say 'every run must be sorted', but the
sortedRunstruct doesn’t track sortedness (onlyvalid), and the current boundary recording happens afterSort(), making runs inherently sorted by construction. To avoid confusion, consider renamingvalidto something likefilled/recorded, and adjust the docstring to reflect that the decision is based on (1) boundaries being recorded and (2) non-overlap (last <= next.first).
// sortedRun holds one flushed run's key boundaries, captured after Sort —
// every run is internally sorted by construction. Runs align 1:1 with
// Collector.dataProviders; an async flush fills its run in the flush goroutine,
// which completes before mergeSortFiles reads it (provider.Wait).
type sortedRun struct {
first []byte
last []byte
valid bool
}
// canReplaySequentially reports whether runs can be replayed one after another
// instead of heap-merged: every run must be sorted and consecutive runs must not
// overlap. An equal boundary key is fine — the heap breaks key ties by run order,
// which sequential replay preserves.
func canReplaySequentially(runs []*sortedRun, providers int) bool {
Cover the single in-RAM run, flushed runs plus an unflushed tail at Load, and an equal-boundary key spanning two runs for all three buffer types (append concatenation order, oldest-appeared first-run-wins, plain duplicate order), each checked against the forced heap path. Claude-Session: https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91
Adds a sorted-input fast path to the ETL collector: when every spilled run's keys arrived already sorted and consecutive runs do not overlap,
Loadreplays the runs sequentially instead of building the k-way merge heap.Motivation
Audit of the 58 collector call sites in the tree found two producers that emit globally sorted keys:
db/state/inverted_index.go—w.indexKeys(key = BE8 txNum, monotone, fed on every state write during exec; the highest-volume ETL user in the node)cl/antiquary/beacon_states_collector.go— the antiquary collectors (ascending slot keys)For these producers the buffer sort was already free (
sortableBuffer.Sortshort-circuits onslices.IsSortedFunc), butLoadstill paid for the merge heap: heapify plus aheapPop+heapPush(with key comparisons) per element. The spill-to-tmp-file and mmap read-back are unavoidable — the data must survive betweenCollectandLoadunder a RAM bound — so this PR removes only the per-element heap work, not the spill.How it works
sortableBuffertracks input sortedness inPutby comparing against the previous key (comparison stops once broken).sortedRun{first, last, sorted}aligned 1:1 with its data providers. For an async flush the boundaries are captured before the buffer is handed to the background sorter, which is why they are only valid when the input arrived sorted.canReplaySequentially(runs, providers)— a pure decision function — requires every run sorted andrun[i-1].last <= run[i].first. When it holds,mergeSortFilesiterates the providers in order; otherwise it falls back to the existing heap merge. Both drivers share the same per-element processing (quit checks,SortableAppendBufferconcatenation,SortableOldestAppearedBufferdedup), so semantics cannot diverge.Sort()regardless of input order, so the common everything-fits-in-RAM collector also skips the heap.No call-site changes: the II writer and the caplin antiquary collectors benefit purely through the collector.
Dup-ordering equivalence
The heap orders elements by
(key, run index)and each run is stably sorted by(key, insertion order). With non-overlapping runs, an equal key at a run boundary drains runicompletely before runi+1— exactly what sequential replay does — and within a run insertion order is preserved by the stable sort in both paths. So the sequential replay's output stream is byte-identical to the heap's, including duplicate keys under DupSort tables. Tests pin this: byte-identicalLoadstreams fast-path vs forced-heap on the same input (equal keys spanning run boundaries included), identical DupSort table contents, and fall-back tests that break sortedness at the secondPut, mid-run, at a run boundary, and scattered.AppendDup for the II load path: deferred
The optional follow-up (using
AppendDupinstead ofPutin the IIloadFuncwhen the stream is sorted) is not included. ForindexKeys, values under one txNum arrive in state-write order, not in byte order, and MDBXAppendDuprequires ascending dup values — so it would change semantics. Left as follow-up work.Benchmarks
Apple M4 Max,
-count=6, benchstat old=main new=this branch (after-side used a bounded-benchtime 5x).LoadMonotoneDupKeysmimics the indexKeys shape (BE8 monotone keys, 4 values per key, ~10 spilled runs).sorted_*take the new sequential path;random_*pin the unchanged heap path;PutOnlyisolates the new per-Putsortedness compare.The
PutOnly/random_100k+26.8% is bounded-benchtime noise (±21%): a recheck at-benchtime 1000xgives700µs vs the 678µs baseline (+3%), consistent withrandom_500k. The honest cost summary: the pure-Putmicrobenchmark pays ~2-12% for the sortedness compare (32-byte keys; real II keys are 8 bytes), while the Load side of the same sorted workload drops 38-55% — in absolute terms forsorted_500k, Put +0.3ms vs Load -5.2ms per pass.Notes
canReplaySequentially, run tracking) — seedb/etl/sorted_replay_test.go.https://claude.ai/code/session_01UYCsHj9HYTJnqUCy8a7W91