fix(concurrency-parallel): pull-based dispatch, crash recovery, timeout semantics - #5
Open
frankstupak wants to merge 1 commit into
Open
Conversation
…y, timeout/retry/abort policy, fast-doubling fibonacci - ParallelManager: shared-FIFO pull dispatch (idle worker takes next task) replaces eager round-robin that head-of-line-blocked short tasks behind a slow task while other workers idled — 1.95x faster on skewed workloads - Worker crash recovery: persistent post-ready error/exit handlers fail the in-flight task loudly and auto-respawn a replacement (previously an unhandled 'error' event crashed the process and in-flight tasks hung) - Fixed activeTasksCount leak (incremented but never decremented on the success path); added getActiveTaskCount/getQueuedTaskCount/getWorkerCount/ getIdleWorkerCount; maxPendingTasks backpressure; timeout now frees the worker when its late result arrives; cleanup rejects queued tasks - Explicit workerScript argument now honored when the file exists (previously silently ignored whenever dist/worker.js existed) - ConcurrencyManager: timeout and retries (declared in ConcurrencyConfig, previously ignored) are enforced; AbortSignal support; genuine fail-fast (other lanes stop pulling tasks after a rejection); executeAllSettled; bounded completedTasks memory - Worker fibonacci: fast doubling O(log n) replaces O(2^n) recursion — fib(42) 4550ms -> 1ms - +19 tests (42 total green), benchmarks/bench.ts included
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.
Lumen Industries uplift #4. The worker pool assigned tasks round-robin without regard to load. Here is what was happening and what changed.
What was wrong
ParallelManager
taskQueuefield existed and was never used — the queue this dispatch model needs.)error/exitlisteners were removed once the worker reported ready, so a post-initerrorevent had no listener (Node throws), and any in-flight task on a dead worker hung until timeout.activeTasksCountonly ever went up. Incremented on dispatch, decremented only in a fallback path that the normal flow never hits.workerScriptargument was silently ignored wheneverdist/worker.jsexisted.fibonacciwas O(2^n) recursion. fib(42) = ~866M calls. The cap says 50; fib(50) is ~40 billion calls — the endpoint would hang for minutes on a legal input.ConcurrencyManager
timeoutandretriesare declared inConcurrencyConfigand were never read. The existing test admitted it: "the current implementation doesn't have built-in timeout handling." It does now, and that test finally tests what its name says.executeLimitedConcurrent/executePriorityQueue, the returned promise rejected but the other lanes kept happily executing side effects behind the back.completedTasksgrew unbounded (ParallelManager had a cap; this class didn't).What's better
maxPendingTasksbackpressure,getActiveTaskCount()/getQueuedTaskCount()/getWorkerCount()/getIdleWorkerCount()observability.maxWorkerRestarts, default 3).cleanup()rejects queued tasks instead of stranding them.timeout,retrieswith error propagation after exhaustion, optionalsignal: AbortSignal, genuine fail-fast (lanes stop pulling after a rejection),executeAllSettled()for per-task outcomes, bounded metrics memory.Numbers (Node 22.22, Xeon E5 T5810, median of 3)
Repro:
npx tsx src/api/concurrency-parallel/benchmarks/bench.ts(script included).Verification
tsc --noEmitclean, eslint clean, rootjestrun: zero new failures vs baseline.— Lumen Industries