Skip to content

fix: bound concurrent pages and cancellation for large cold scans - #228

Open
6tamichael-boop wants to merge 4 commits into
wraith-protocol:developfrom
6tamichael-boop:cloud-fixer/sdk-208-b1-1
Open

6tamichael-boop wants to merge 4 commits into
wraith-protocol:developfrom
6tamichael-boop:cloud-fixer/sdk-208-b1-1

Conversation

@6tamichael-boop

Copy link
Copy Markdown

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

  • [MODIFY] src/chains/stellar/announcements.ts
    • Add MAX_COLD_SCAN_PARALLELISM = 8 and clamp the parallelism option to [1, MAX_COLD_SCAN_PARALLELISM] (non-finite/fractional hints fall back to 1). Each chunk keeps one getEvents page in flight and one buffered item in the merge, so an unbounded hint no longer means unbounded pending RPC work.
    • The option's JSDoc now states the clamp; the constant is module-level (not re-exported from the package entry, so the public API surface is unchanged).

Bound the merge buffer, fix cancellation

  • [MODIFY] src/chains/stellar/announcements.ts — mergeOrdered
    • Wrapped the merge loop in try/finally and return every chunk iterator on exit, including ones that never yielded. Previously a consumer break/.return() only unwound the delegated chain, leaving other chunk generators suspended with a page outstanding.
    • Documented the existing backpressure contract explicitly: exactly one item buffered per chunk, and a chunk is pulled again only after its previous item was consumed, so memory is O(chunks) rather than O(scan size).

Regression tests

  • [MODIFY] test/chains/stellar/announcements.test.ts
    • caps cold-scan parallelism so in-flight chunks stay bounded: an oversized parallelism: 10_000 produces exactly MAX_COLD_SCAN_PARALLELISM distinct chunk requests.
    • mergeOrdered closes every chunk iterator when the consumer cancels: five chunk generators, break after one item, all five finally blocks 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

  • [MODIFY] test/chains/stellar/bench/scan.bench.ts
    • New Stellar cold-scan backpressure section: a correctness test draining MAX_COLD_SCAN_PARALLELISM chunks with a slow consumer, and a matching bench case, so the bound is exercised under the existing bench harness.

Docs

  • [MODIFY] docs/chains/stellar-streaming-scan-pipeline.md
    • New "Bounding parallel cold scans" section covering the chunk cap, the O(chunks) merge buffer, and the cancellation teardown added here.

Verification Results

Changes authored via GitHub Contents/Git API (no local clone or sandbox).
`pnpm test`, `pnpm exec vitest bench` and typecheck were not executed in this
environment; the new tests/bench are provided but unrun here. The public API
surface is unchanged (`MAX_COLD_SCAN_PARALLELISM` is not re-exported from the
package entry), so the api-extractor reports do not need regeneration.
Acceptance criteria mapping — see the table below.
Acceptance Criteria Status
Bound concurrent pages and buffered announcements ✅ MAX_COLD_SCAN_PARALLELISM clamp + one-item-per-chunk merge buffer
Verify memory stays bounded while consuming slowly ✅ Slow-consumer lead-bound regression test
Confirm cancellation stops all outstanding requests ✅ mergeOrdered finally returns every chunk iterator + regression test
Add a benchmark and a regression test ✅ Stellar cold-scan backpressure bench section + 3 tests

Closes #208

@drips-wave

drips-wave Bot commented Sep 25, 2026

Copy link
Copy Markdown

@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! 🚀

Learn more about application limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Wave 9] Add backpressure tests for large cold scans

1 participant