Bounded concurrency for the gig/repin/event sweeps + fix the dead webhook retry helper (#236–#239) - #359
Merged
meshackyaro merged 11 commits intoAug 31, 2026
Conversation
`mapWithConcurrency(items, n, worker)` runs `worker` over `items` with at most `n` in flight, returns `PromiseSettledResult`s in input order, and never rejects. Replaces the `for..of await` sweeps whose runtime was the sum of every item's latency (trustflow-protocol#236, trustflow-protocol#237, trustflow-protocol#238).
…rlapping sweeps (trustflow-protocol#236) `runOnce()` fanned out `expire()` one gig at a time; each `expire` appends an outbox row the relay delivers with retries, so a big batch with a slow endpoint serialised all that latency and could outrun the sweep interval. Now bounded-concurrent (GIG_EXPIRY_SWEEP_CONCURRENCY, default 8), a failed expire is counted+logged not fatal, and a `sweeping` flag skips a tick while a previous sweep is still running. Closes trustflow-protocol#236
…low-protocol#237) `runOnce()` awaited `pinningService.reconcile(cid)` serially; each call makes per-provider network requests, so sweep time scaled with pin count x latency. Now bounded-concurrent (IPFS_REPIN_SWEEP_CONCURRENCY, default 8) with the per-CID try/catch kept inside the worker so one bad CID stays isolated, plus the same overlapping-sweep guard. Closes trustflow-protocol#237
…rve per-escrow order (trustflow-protocol#238) New `processEventBatch()` (used by `ingestEvents` and `ingestSingleLedger`): phase 1 runs every id-less event (escrow_created, unknown types) strictly in original order so no later keyed event can observe a missing escrow; phase 2 groups the `topic[1]`-keyed events per escrow and runs the groups in parallel (EVENT_PROCESSING_CONCURRENCY, default 8), sequential within a group. Full concurrency-safety analysis is in the method's doc comment. Closes trustflow-protocol#238
trustflow-protocol#239) `retry.helper.ts` was dead code — `WebhookService.sendWithRetry` re- implemented the loop inline. It now calls `withRetry(...)`, and `isRetryable` no longer classifies any message containing the digit 5 as retryable: it matches transient network errors and a real HTTP 5xx status (the bare 3-digit message `send()` throws, or `HTTP 5xx`). `withRetry` gained a `shouldRetry` predicate so a 4xx stops immediately. Closes trustflow-protocol#239
|
@stephanieoghenemega-eng 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! 🚀 |
6 tasks
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.
Four issues, all about the background workers processing their sweeps one item at a time. A shared
mapWithConcurrencyhelper (commit 1) is applied to the three sweeps; #239 is a separate dead-code + bug fix.#236 —
GigExpiryWorkerServicesweeps sequentiallyrunOnce()awaitedgigService.expire(gig.id)per gig in a plain loop; eachexpire()appends an outbox row the relay delivers with up-to-3 retries, so a batch of simultaneously-expiring gigs against a slow endpoint serialised all of that latency and could still be running when the next tick fired.GIG_EXPIRY_SWEEP_CONCURRENCY, default 8).expireis counted + logged, not fatal to the sweep.sweepingflag: a tick is skipped (and logged) while a previousrunOnce()is still in flight. (The Redis lock already covered the multi-instance case; this covers a single instance whose sweep outruns its interval.)#237 —
RepinWorkerServicereconciles sequentiallySame shape:
await pinningService.reconcile(cid)per degraded/failed pin, each doing per-provider network calls. Now bounded-concurrent (IPFS_REPIN_SWEEP_CONCURRENCY, default 8), with the per-CIDtry/catchkept inside the worker so one unreachable provider stays isolated to its CID, plus the same overlapping-sweep guard.#238 —
EventIngestionServiceprocesses events sequentiallyingestEvents/ingestSingleLedgerloopedawait processEvent(event)for every fetched event, so a 100-ledger window's events processed fully serially each 5s poll tick.New
processEventBatch(events)(used by both):topic[1](escrow_created, unknown types) runs strictly in original order.escrow_createdcarries no escrow id, so it can't be correlated to a specific laterescrow_funded; running all id-less events first guarantees no keyed handler observes a missing escrow.topic[1]-keyed events are grouped per escrow id; groups run in parallel (EVENT_PROCESSING_CONCURRENCY, default 8), sequential within a group, so same-escrow ordering (funded → released) is preserved while independent escrows don't block each other.Full concurrency-safety analysis is in the method's doc comment. Test asserts a slow
escrow_fundedfor escrow A does not delay escrow B and does not reorder A's own events.#239 —
retry.helper.tsis dead code +isRetryable()bugwithRetry/isRetryablewere exported but never imported —WebhookService.sendWithRetryre-implemented the loop inline. AndisRetryablediderror.message.includes('5'), matching the digit 5 anywhere (a URL like/v5/,512 bytes, port5000).sendWithRetrynow callswithRetry(() => this.send(...), retries, 1000, isRetryable)— one implementation.isRetryable(error: unknown)matches transient network errors (ECONNREFUSED,ETIMEDOUT, …) and a real HTTP 5xx — the bare 3-digit messagesend()throws (new Error(String(statusCode))), or"HTTP 5xx …". A 4xx or a message that merely contains a 5 is not retried.withRetrygained ashouldRetrypredicate so a non-retryable failure stops immediately instead of burning all attempts.retry.helper.spec.tsadded.Not built or run in this environment (per your instruction). New/changed code targets
tsc --noEmitand the repo's prettier/eslint config; tests follow the existing*.spec.tspatterns (jest.Mocked<Pick<…>>,fakeLock,flushPromises,Test.createTestingModule).Closes #236, closes #237, closes #238, closes #239