Repository navigation
perf(ingestor): yield between committed retention batches - #2153
Conversation
|
Review feedback addressed (commit
|
|
I measured this on another machine. The gains were larger than in your CI run, and retention itself was not slower. Benchmark.
singleHot shows no change, as you expected. That fixture is one transaction with nothing to yield between. Not measured on staging. I'm running this PR on a staging instance, but retention only runs there once a day, so the benchmark above is the evidence. 🤖 Generated with Claude Code |
|
The product change is sound. The yield runs after each batch's Two things before merge:
I'll merge after that. |
|
Thanks for the concrete merge requirements. I checked current upstream: the post-commit yield is still absent, so this remains useful independently of the PostgreSQL work. The next revision should:
The independent benchmark results are useful corroboration, but they should not be pooled with the differently configured paired study. The requested rebase and report cleanup have not been applied in this review pass. |
|
I’m applying the requested rebase onto current upstream I’m preserving the original |
323b18d to
3074323
Compare
|
Review feedback addressed (commit
Post-push Chromium validation passed 148 checks with three explicit fixture-dependent skips against the unchanged upstream server/frontend. Fresh CI passed on |
Fixes #2151. Related to #2146.
Yield to runnable goroutines after each committed full retention batch, once the writer lock is released. This gives the serial ingest consumer another opportunity to submit its next write. Batch size, cutoff and deletion semantics stay unchanged; dense fanout within one transaction remains unbounded.
The branch is rebased onto upstream
b9593f71745b856651cfd6d6c80bb1f3f091e13e. The duplicate test commit99c2722dis dropped because #2160 is already present upstream asa5b60ca5. The runtime and regression-test patch is unchanged from the previously reviewedb70a9f6b.The raw JSON profile is omitted from this branch. The Markdown report links to the immutable original profile; the linked profile was fetched and verified against its original Git blob.
Performance scope:
9dbc287versusb70a9f6, with 2,048,000 observations, 50 ingest events/s and 20 HTTP requests/s.Local validation on Windows with Go 1.27.2, CGO enabled, and package parallelism limited to one:
typical16,dense256,singleHot).GOMAXPROCS=4. An initial run during competing test jobs reached its 600-second limit; after those jobs finished, the retry passed with the same 600-second limit and all assertions intact.Post-push Chromium validation passed 148 checks with three explicit fixture-dependent skips. The server and frontend source are identical to upstream
b9593f71; this browser pass checks the existing application journeys and does not measure retention performance. Fresh CI passed on30743237, including the full Go job, ingestion race detector, all three browser shards and the two-architecture build.