fix: bound concurrent pages and cancellation for large cold scans - #228
Open
6tamichael-boop wants to merge 4 commits into
Open
6tamichael-boop wants to merge 4 commits into
6tamichael-boop wants to merge 4 commits into
Conversation
|
@6tamichael-boop Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Overview
Cold scans can split the ledger range into parallel chunks and interleave them with
mergeOrdered, but nothing bounded how many chunks a caller could request, and cancelling a merge did not close the chunk generators that had not yielded yet. This change caps the parallel chunk count, documents the one-item-per-chunk merge buffer, and makes cancellation tear down every chunk iterator. It also adds the regression tests and benchmark the issue asks for.Related Issue
Closes #208
Changes
Bound concurrent pages
src/chains/stellar/announcements.tsMAX_COLD_SCAN_PARALLELISM = 8and clamp theparallelismoption to[1, MAX_COLD_SCAN_PARALLELISM](non-finite/fractional hints fall back to 1). Each chunk keeps onegetEventspage in flight and one buffered item in the merge, so an unbounded hint no longer means unbounded pending RPC work.Bound the merge buffer, fix cancellation
src/chains/stellar/announcements.ts—mergeOrderedtry/finallyand return every chunk iterator on exit, including ones that never yielded. Previously a consumerbreak/.return()only unwound the delegated chain, leaving other chunk generators suspended with a page outstanding.Regression tests
test/chains/stellar/announcements.test.tscaps cold-scan parallelism so in-flight chunks stay bounded: an oversizedparallelism: 10_000produces exactlyMAX_COLD_SCAN_PARALLELISMdistinct chunk requests.mergeOrdered closes every chunk iterator when the consumer cancels: five chunk generators, break after one item, all fivefinallyblocks run.mergeOrdered bounds how far each chunk runs ahead of a slow consumer: interleaved keys and a deliberately slow consumer; per-chunk lead stays within one buffered item plus the in-flight one.Benchmark
test/chains/stellar/bench/scan.bench.tsStellar cold-scan backpressuresection: a correctnesstestdrainingMAX_COLD_SCAN_PARALLELISMchunks with a slow consumer, and a matchingbenchcase, so the bound is exercised under the existing bench harness.Docs
docs/chains/stellar-streaming-scan-pipeline.mdVerification Results
MAX_COLD_SCAN_PARALLELISMclamp + one-item-per-chunk merge buffermergeOrderedfinallyreturns every chunk iterator + regression testStellar cold-scan backpressurebench section + 3 testsCloses #208