Skip to content

fix(compression): drain the worker pool queue when worker construction throws - #12755

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.51from
insoln:fix/compression-worker-pool-queue-jam
Sep 29, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.51from
insoln:fix/compression-worker-pool-queue-jam

Conversation

@insoln

@insoln insoln commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Prevents the compression worker pool from retaining queued request bodies when worker construction throws synchronously.

This is deliberately a containment fix: it removes the queue-retention failure mode and makes fail-open behavior observable. Compression still returns the original body when no worker can be created.

Changes

  • Drain every queued job fail-open when worker construction throws.
  • Permanently short-circuit known structural spawn failures only when no worker has been created; preserve existing healthy workers when adding capacity fails.
  • Retry transient and unknown spawn failures on the next request wave.
  • Settle in-flight work on worker errors, exits, timeouts, postMessage throws, and pool shutdown.
  • Add bounded, rate-limited, sanitized fail-open warnings.
  • Safely handle unknown thrown values without allowing error formatting to reject the fail-open path.
  • Pass a URL object to the LLMLingua worker constructor on Node.js 21+.
  • Cover LLMLingua spawn and postMessage failures, queue continuation, and worker respawn.
  • Register the regression suite in Stryker's test input set without expanding mutation scope.

Validation

  • Focused regression suite — 23 passed.
  • Focused compression and LLMLingua suites — 53 passed.
  • Core typecheck, formatting, and diff validation — passed locally.
  • Specialized code and test-coverage reviews found no remaining actionable defects.
  • Both Copilot findings were addressed: preserving existing healthy worker capacity and using the canonical upstream PR link in the changelog.

The release branch is currently red independently of this change.

⚠️ base-red inherited: #12732

Tests Added

  • tests/unit/compression/compression-worker-pool-jam.test.ts

Coverage includes structural/transient/unknown spawn failures, populated queue draining, broken-pool short-circuiting, preservation of healthy workers, runtime failures, shutdown settlement, safe handling of unusual thrown values, notifier rate limiting/sanitization, and LLMLingua failure recovery.

Follow-up issues

These root-cause repairs are intentionally separate from this queue-retention containment PR. The runtime must both resolve the worker entry and provide its required packaged dependencies.

@insoln

insoln commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up issues filed for the out-of-scope root causes:

@JasonBroderick

Copy link
Copy Markdown
Contributor

Independent field confirmation from a self-hosted Docker deployment (standalone build of release/v3.8.51 at c3945a7, Node 26), plus the numbers, in case they help get this merged.

Symptom. With #12542's idle-terminate applied, worker threads stayed flat but the main-isolate heap still grew by roughly 5 MB per Claude Code request (about 240 MB to 1.8 GB in 25 minutes of a busy session), then sat at the admission-guard shed line for 90 minutes with almost no traffic. large_object_space held 556 MB of the 845 MB heap.

Retainer path (heap snapshot of a cloned instance after six 420 KB requests, streamed over the inspector):

string (bodyString, 945 KB)
  <- Object (ProviderRequestPrepared) <- closure `latest`/`capture` (createPreparedRequestLogger)
  <- closure `onEngineStep` (chatCore compressionOptions)
  <- Array `queue` [0..5] <- CompressionWorkerPool instance
  <- module context of runCompressionInWorker <- global `__omnirouteRequestShutdown` <- (Global handles)

Pool state in the same snapshot: queue = 6 jobs (one per request, each holding body, originalBody, options, resolve, onEngineStep), workers Set = empty. So every eligible request's job sat in the queue forever.

The throw (inspector Debugger.setPauseOnExceptions, one request):

Error: Cannot find module '/app/open-sse/services/compression/compressionWorker.js'
    at moduleContext (/app/.build/next/server/chunks/[turbopack]_runtime.js:403:19)
    at a.spawn (...)  at a.dispatch (...)  at run (...)

spawn() throws synchronously before queue.shift(), run()'s executor rethrows so the caller falls open, and the pushed job is never removed. Exactly the failure mode this PR describes. (Separately, new Worker(path) from the inspector resolves the path fine but the worker then dies with Cannot find package 'uuid', so #12796 is real too.)

Result with this PR applied as a local patch on top of #12542 (rebuilt from source, same Docker path): cloned instance after the same six requests shows queue = 0, broken = true, no request strings retained, and the rate-limited [compression] worker path failed open line makes the condition visible for the first time. Switched production to it an hour ago; heap is flat so far where it was climbing before.

Quick bridge for anyone hit by this before the PR lands: setting the compression enabled setting to false stops the leak immediately (5 s settings cache, no restart), since run() is never reached. Compression is not working in the standalone build anyway (#12797, #12796), so nothing is lost.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

This containment fix (spawn-throw queue drain + fail-open observability) is solid and well tested
(23/23 passing on its own head). Two things before merge: (1) the tip has moved since your
merge-base — #13093 already ships the LLMLingua URL-object fix this PR also includes (item 6 in
your description), and #13091 changed remove()'s signature (dropped the 2nd bool param) which
your close() still calls with — both are mechanical but need a real rebase, not just an
auto-merge. (2) PR #13637 (same review group) rewrites the same fail()/spawn() methods for a
different failure mode (runtime worker fault vs. your synchronous construction-throw case) with an
incompatible design — it adds reject() so the caller restores in-process compression, while this
PR keeps the pool's "never reject" contract. We'd like to land #13637 first (it fixes the
user-visible #13145 symptom) and have this PR rebase on top, adding only the construction-failure
protection layer without reverting the reject-based runtime-fault path. Let us know if you'd rather
coordinate the order differently.

@insoln

insoln commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

I validated the ordering against #13637 rather than guessing at it: I replayed this PR on top of #13637's head in a scratch branch and resolved every conflict by hand. The two fixes reconcile cleanly and neither has to give anything up — so the order you proposed (#13637 first, this PR rebased on top) works, and this branch is deliberately left on release/v3.8.51 so its diff stays free of #13637's unmerged commits.

How the two intents combine in compressionWorkerPool.ts:

  • Construction path stays fail-open. spawnOrFailOpen() drains the queue with unchanged(...) only when no worker is left to drain it. A capacity-spawn failure while a healthy worker is still running keeps the backlog for that worker — finish() re-enters dispatch(). broken is set only for a structural failure with no worker ever created, so an unknown error (including ERR_WORKER_INIT_FAILED) is retried on the next wave instead of disabling compression forever.
  • Runtime path stays reject-based. fail(slot, reason, retryInProcess = true) delegates to abort(slot, new CompressionWorkerError(reason, retryInProcess)). A dispatch timeout passes retryInProcess: false — the worker already spent its full budget, so repeating that CPU-bound work on the main event loop would stall every other in-flight request. Every other fault (thread error, exit, engine throw, postMessage throw) failed fast without doing the work, so the caller can restore compression in-process.
  • One notification point, no double logging. abort() emits notifyCompressionFailOpen(...) before rejecting, which closes the zero-log condition behind issue build(deps): bump actions/github-script from 7 to 8 #2 for runtime faults too. strategySelector notifies only for errors that never reached the pool — e.g. the dynamic import() of the pool module itself failing with Turbopack's MODULE_NOT_FOUND.
  • reject is stripped from the wire job alongside the other non-cloneable fields: postMessage uses structured clone, and leaking any function throws DataCloneError before the worker ever sees it. postMessage itself is guarded so a non-cloneable payload cannot strand the slot until its timeout.

The retry-vs-degrade decision moves into open-sse/services/compression/workerFailureRecovery.ts, which keeps strategySelector.ts under its frozen file-size baseline and the pool lazily loaded (a static class import would pull worker_threads eagerly).

Tests follow the merged contract: runtime faults assert a CompressionWorkerError carrying the right retryInProcess value (error/exit true, timeout false) while still asserting the warn line, and the spawn-throw fail-open cases are unchanged. One case is new — a queued job must survive a capacity-spawn failure and still be picked up by the surviving worker.

Validation of the reconciled result in a clean npm ci worktree based on #13637's head:

  • compression unit tests: 1537/1537 pass
  • Vitest: 473/473 pass
  • typecheck:core, ESLint, complexity ratchets, Prettier: pass

Two gate failures reproduce unchanged on #13637's head itself and touch no file in this diff: check:file-size (open-sse/utils/stream.ts 3114 > 3098 — the known #12732 entry deliberately left un-rebaselined) and check:cycles (the src/lib/db/{models,providers,providers/deletion}.ts cycle).

Merge #13637 whenever it's ready and I'll push the rebased version of this PR right after — the resolution is already prepared, so it's a fast turnaround. Happy to invert the order instead if you'd rather land this one first.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Segurando: conflitante com a base e com veredito ⭐3, que precisa de reconfirmação.

A PR está conflitante com release/v3.8.51, e o veredito de ⭐3 do plan-file foi medido contra uma base bem anterior. Como o caminho de compressão recebeu vários merges hoje (#13423 no gate do worker, #13521 no RTK, #13915 no cap do Lite, #12905 no preamble), o conflito tende a crescer enquanto ela espera.

Sugestão: rebase sobre o tip atual e reconfirmação do veredito. Vale especialmente verificar se o caso que ela ataca — fila do worker pool não drenada quando a construção do worker lança — ainda se reproduz depois da #13423, que mexeu justamente em isStrictlySerializable, o gate que decide se a compressão vai para o worker ou roda inline. Se o gate mudou de comportamento, a condição de corrida pode ter mudado junto.

Se você confirmar que o defeito persiste no tip, com um repro, eu levo para merge.

Replays cheaper-inference#12755 on top of cheaper-inference#13637 so both fixes survive intact.

A synchronous spawn failure left the queue stranded: nothing else calls dispatch() again, so queued jobs and their full request bodies were retained for the lifetime of the process. Queued jobs now reject as retryable when no worker is left to drain them, while a capacity-spawn failure with a healthy worker keeps the backlog for that worker.

Preserve cheaper-inference#13637's reject-based runtime-fault contract: fast faults retry compression in-process, while a dispatch timeout degrades to the uncompressed body after spending the worker budget. Keep fail-open reporting, strip non-cloneable callbacks from wire jobs, and guard postMessage failures.

Add an end-to-end regression proving the cheaper-inference#13423 shared-subobject shape remains worker-eligible and falls back to in-process compression after synchronous MODULE_NOT_FOUND.
@insoln
insoln force-pushed the fix/compression-worker-pool-queue-jam branch from 286006e to 656e716 Compare September 22, 2026 18:12
@insoln

insoln commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current tip and reconfirmed the defect post-#13423 (answering your 2026-09-18 review).

Rebase. The branch is now a single commit (656e7169) replayed on top of the current release/v3.8.51 tip; conflicts against the #13637 squash were resolved in favor of the merged reject-based contract. A range-diff against the previously prepared reconciliation shows no unintended changes.

Repro that the defect persists after #13423. New e2e regression in tests/unit/13145-compression-worker-failure-fallback.test.ts:

  1. builds a body with the exact fix(compression): track only the recursion path in isStrictlySerializable (#13154) #13423 shared-subobject shape (two sibling branches referencing the same non-cyclic content part) and asserts isCompressionWorkerEligible(...) === true — the request really routes through the worker;
  2. injects a worker factory that throws synchronously (MODULE_NOT_FOUND, the Turbopack standalone signature);
  3. asserts the pool drains the stranded queue by rejecting every queued job with a retryable CompressionWorkerError, and applyCompressionAsync then restores compression in-process: spawns === 1, result.compressed === true, originalTokens > compressedTokens.

On the unfixed pool the same input strands the queue forever (nothing re-enters dispatch()), retaining one full request body per job for the process lifetime.

Relationship to #13637. This PR only adds the construction-failure layer; the merged runtime-fault contract is preserved: fast faults (spawn throw, worker error/exit, engine throw, postMessage clone failure) reject with retryInProcess: true so the caller retries in-process; a dispatch timeout keeps retryInProcess: false (degrade uncompressed + logged) so a burned CPU budget is never re-run on the event loop. Only structural spawn failures (MODULE_NOT_FOUND / ERR_WORKER_PATH / …) with zero workers ever created mark the pool permanently broken; transient ones (e.g. ERR_WORKER_INIT_FAILED) are retried next wave, and a capacity-spawn failure while a healthy worker exists keeps the backlog for that worker.

Validation

Inherited CI failures (not from this PR). API Route Typecheck, Fast Quality Gates, Docs Gates (fast-path) and the unit-test shards fail on files this PR never touches — open-sse/executors/auggie.ts (TS2769/TS18047), src/app/api/v1/combos/projectCombo.ts (TS2459/TS2724), check:deps (@opencode/plugin), docs-counts drift (181 vs 178 migrations), and provider golden/alias/catalog tests. The same workflows were already red at the previous head 286006eb (2026-09-18) and at 9cfc18e (2026-09-16), so these come from the base tip, not from this change. Happy to split any of them into a separate base-red PR if useful.

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.

3 participants