Skip to content

fix(meteor-promise): create pool fibers without inherited async context - #8

Open
jbaczuk-qualia wants to merge 1 commit into
make-compatible-2.8from
jbaczuk/fiber-pool-no-inherited-context
Open

jbaczuk-qualia wants to merge 1 commit into
make-compatible-2.8from
jbaczuk/fiber-pool-no-inherited-context

Conversation

@jbaczuk-qualia

Copy link
Copy Markdown

Problem

meteor-promise runs every Promise.asyncApply / fibered .then callback on a fiber from a shared pool. Between jobs each pool fiber parks itself with Fiber.yield() in its own async scope. The fibers fork gives every fiber an AsyncResource whose AsyncLocalStorage stores are copied from the creator once, so a pool fiber created while method M was running carries M's OpenTelemetry context forever, and every idle yield it ever makes is attributed to M.

On qualia-impacttitle this produced ~11k/s of meteor_fiber_yields_total labelled refreshDocs, IncomingWires.OrderMatches.refresh, etc. while none of those methods were running (the process had created 205 fibers in 12 hours; all of the traffic was the customer's GraphQL poller going through the pool).

Fix

  • Create pool fibers inside a module-load-time AsyncResource (PRISTINE_ASYNC_SCOPE) so they inherit no stores at all.
  • Set fiber._meteorPromisePoolIdle around the idle Fiber.yield() so Fiber.yield instrumentation can label the park explicitly (qualia's prom-client will count it as fiber_pool/idle; companion PR in the qualia repo).

Job callbacks still run inside their per-job AsyncResource (entry._ar) and see the submitter's context as before. Meteor environment variables were already unaffected: fibers_async.js resets _meteor_dynamics on fiber start, and jobs read them through the job scope.

Test

test/tests.js › "fiber pool async context": grows the pool under ALS store creator, then runs 50 trivial jobs under user and asserts no yield sees creator, jobs see user, and the flagged idle yields see no store. Fails against the previous fiber_pool.js with a yield during the user phase saw the creator's store; passes with this change. Runs only on the native-Promise pass (the promise polyfill's asap scheduler batches unrelated reactions under one async context).

Verified on Node 18.16.1 (same as prod).

Versions

meteor-promise 0.9.1-2 → 0.9.1-3, promise package 0.12.2 → 0.12.3 (Npm.depends updated).

🤖 Generated with Claude Code

Pool fibers are shared between unrelated callers, but each one was created
inside whatever AsyncLocalStorage context happened to be active when the
pool needed to grow, and the fibers fork copies that context onto the
fiber's own AsyncResource. The fiber's own scope is what is active while it
parks itself with Fiber.yield() between jobs, so every idle yield reported
the creator's context (e.g. the OpenTelemetry root span of a method that
finished days ago) for the rest of the process. In prod this showed up as
~11k/s of meteor_fiber_yields_total attributed to methods that were not
running.

- Create pool fibers inside a module-load-time AsyncResource so they
  inherit no stores from their creator.
- Flag the fiber with _meteorPromisePoolIdle around the idle yield so
  instrumentation wrapping Fiber.yield can tell the park apart from a real
  yield.
- Add a test that fails against the previous fiber_pool.js.

Job callbacks still run inside their own per-job AsyncResource and are
unaffected.

meteor-promise 0.9.1-2 -> 0.9.1-3, promise 0.12.2 -> 0.12.3.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
// Call Fiber.yield() to await further instructions.
// Call Fiber.yield() to await further instructions. Flag the fiber as idle so that
// instrumentation wrapping Fiber.yield can tell this park apart from a real yield.
fiber._meteorPromisePoolIdle = true;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flag this fiber as a idle pool fiber before yielding so we can detect it in the app

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant