Skip to content

refactor(client): share startup attempt bookkeeping between clients - #53240

Merged
kitlangton merged 2 commits into
v2from
share-contender-pool
Oct 5, 2026
Merged

kitlangton merged 2 commits into
v2from
share-contender-pool

Conversation

@kitlangton

@kitlangton kitlangton commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Why

Both ensure() loops track their startup attempts with the same four variables (contenders, failure, spawnDelay, lastSpawn) and the same 25 lines of rules. #50784 had to change those rules identically in both clients, and nothing but review kept them in sync.

What Changes

contenderPool(timing) in service-contender.ts owns that bookkeeping. Each client keeps its own IO (announcing, spawning, terminate, stop) and calls the pool at the same points as before.

Pool method Replaces, in both clients
serviceAnswered() spawnDelay = timing.spawnDelay when a registered service answers
evict(pid) release and drop the replaced owner's attempt and finished ones, failure = undefined, lastSpawn = 0
recruitNow() lastSpawn = Date.now() - spawnDelay after killing an unresponsive owner
reap() collect finished attempts, keep the first failure, back off on a clean exit, return the failure once no attempt is alive
shouldRecruit(registered) wait one spawn delay for an unanswered registration; at most two attempts; none after a failure
add(contender), releaseAll(), failure() contenders.add + lastSpawn = Date.now(), the final release, the timeout error
// both clients, no registered service answered
const failed = pool.reap()
if (failed !== undefined) throw failed          // Effect: return yield* Effect.fail(failed)
if (pool.shouldRecruit(registration.info !== undefined)) {
  announce("missing")
  pool.add(await spawnContender())
}

One ordering moved: the old loop started the "unanswered registration" clock before reaping, and the pool starts it inside shouldRecruit(), after reap(). Nothing reads the clock in between, and the only path that skips it is the fatal one, which leaves ensure().

Scope

Stacked on #53237. A pure move of #50784's rules, with no behavior change intended. Next: the decision ensure() makes when a registered service answers.

Verification

cd packages/client
bun typecheck
bun run test test/service-contender-pool.test.ts test/service.test.ts test/promise-service.test.ts test/service-reconnect.test.ts --rerun-each 3
  • 171/171 across three runs of each. The black-box service tests are unchanged. One earlier full run had two intermittent failures in Effect replacement tests. They did not come back in 399 later runs on this branch, and the same tests also pass on the base.
  • New service-contender-pool.test.ts exercises the pool with real short-lived bun -e processes: retained first failure, eviction, the unanswered-registration delay, the two-attempt limit, capped backoff, and the backoff reset when a service answers.
  • Mutation check: dropping the "no new attempts after a failure" rule from the pool fails fix(client): preserve background service startup failures #50784's three tests in both the Effect and Promise suites, plus the pool test. That shows both loops run through the shared rules.

Stack

  1. fix(client): preserve background service startup failures #50784
  2. refactor(client): share the service health probe between clients #53237
  3. refactor(client): share startup attempt bookkeeping between clients #53240 👈 current
  4. refactor(client): share the registered service decision between clients #53241

@kitlangton
kitlangton force-pushed the share-contender-pool branch from 8d52db1 to 9907f16 Compare October 5, 2026 06:00
@kitlangton
kitlangton merged commit e2d5400 into v2 Oct 5, 2026
9 checks passed
@kitlangton
kitlangton deleted the share-contender-pool branch October 5, 2026 15:18
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