Repository navigation
fix: bound concurrent pages and cancellation for large cold scans - #228
6tamichael-boop wants to merge 6 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! 🚀 |
|
The iterator cleanup test only proves return() is called on paused generators. It does not cancel an in-flight fetch, and initial next() calls are still awaited sequentially. Please propagate AbortSignal to RPC requests and test cancellation with blocked fetches. |
|
CI is approved and running now. Please format src/chains/stellar/announcements.ts and test/chains/stellar/announcements.test.ts. The cancellation and sequential initial fetch issues from my earlier review also remain. |
…initial fetch - Thread an AbortSignal through fetchAnnouncementsStream, mergeOrdered and every Soroban/Horizon fetch so pending RPC and pagination work is cancelled. - Start the first page in parallel and cap cold-scan parallelism so in-flight chunk requests stay bounded. - Move the abort/ordering coverage into test/chains/stellar and format.
|
@truthixify Both issues from your review are addressed, plus formatting.
Verified locally on a checkout of the branch merged with Head is now |
|
@truthixify Verified head
Could you re-review? |
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