refactor: switch worker timers from setInterval to recursive setTimeout#2807
Open
andsai wants to merge 1 commit into
Open
refactor: switch worker timers from setInterval to recursive setTimeout#2807andsai wants to merge 1 commit into
andsai wants to merge 1 commit into
Conversation
…ut (junobuild#2522) Following the pattern from oisy-wallet#9706: each worker now schedules the next tick only after the previous async callback resolves, so the sync cannot overlap itself. With setInterval, a callback slower than the interval would queue overlapping calls (race conditions, duplicated network requests). Applied across all 8 worker timers: - workers/auth.worker.ts - workers/icp-cycles-rate.worker.ts - workers/cycles.worker.ts - workers/exchange.worker.ts - workers/hosting.worker.ts - workers/wallet.worker.ts - workers/workflows.worker.ts - workers/monitoring.worker.ts Each start function now early-returns if a timer is already running (`nonNullish(timer)`), matching the OISY pattern. clearInterval is swapped for clearTimeout. Now-redundant overlap guards are removed: - `let syncing = false` plus its guard/assignments in icp-cycles-rate, cycles, exchange, hosting, monitoring workers. - `syncing: Record<...,boolean>` plus per-key guard/assignments in wallet and workflows workers. Recursive setTimeout guarantees serial execution by construction, so the manual flags become dead code.
Author
|
Hi @peterpeterparker 👋 Picked this one off the open list — the OISY reference made it a clean apply across all eight workers. Mirrored the exact pattern from #9706 there. The Buon vento ⛵ |
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.
Closes #2522.
Follows the pattern from oisy-wallet#9706 — applied across all 8 worker timers in the Console (not just
auth.worker.ts).Why
With
setInterval, a callback slower than the configured interval queues overlapping calls. The workers tried to defend against this with manuallet syncing = falseguards (5 of 8 files) or per-keyRecord<…, boolean>maps (wallet + workflows). RecursivesetTimeoutmakes that guarantee structural: the next tick is only scheduled after the previous async callback resolves.What
auth.worker.ts,icp-cycles-rate.worker.ts,cycles.worker.ts,exchange.worker.ts,hosting.worker.ts,wallet.worker.ts,workflows.worker.ts,monitoring.worker.ts.startXxxTimernow early-returns whennonNullish(timer)is true (matches OISY) so the timer can't be double-started.clearInterval→clearTimeout.let syncing = falseplus its guard/assignments inicp-cycles-rate,cycles,exchange,hosting,monitoring.syncing: Record<IcrcAccountText, boolean>inwallet.worker.ts.syncing: Record<WorkflowsIdbKey, boolean>inworkflows.worker.ts. Its companionWorkflowsIdbKeyimport is auto-removed by ESLint as unused.Verification
npm run check:all— green.npx vitest --config ./vitest.frontend.config.ts --dir src/frontend run— 65 tests pass across 8 files.grep -rn 'setInterval' src/frontend/src/lib/workers/returns only the docstring references in the new// Recursive setTimeout (not setInterval)comments.One subtle side effect worth flagging
For
wallet.worker.ts, each account currently callsstartTimerWithAccountand stomps on the module-leveltimer— so before this PR, thestopTimer()call only cleared the last account's interval and earlier ones leaked. With recursivesetTimeout, thenonNullish(timer)check inside each closure naturally letsstopTimer()drain all account lineages on the next tick (they each seetimer === undefinedand don't reschedule). Behavior change is positive but worth a maintainer eyeball — happy to revert that side of the change if you'd prefer to keep the existing semantics and address per-account timer tracking in a separate PR.