Give each forked worker its own redis lock contender identity

Merged
MA
2d ago
+338-20

Fixes FINDING-030 from the reflex 0.11.0a1 pre-release testing (artifacts on branch claude/vibrant-pasteur-3921g3, prerelease-testing/runs/2026-11-11-v0.11.0a1/redis_runtime/ and verify_rds/).

Defect

With REFLEX_OPLOCK_ENABLED=true and several production backend workers (fork, the Linux default), an event waiting for a token's lock stalled about a tenth of lock_expiration (~1 s by default) under contention, contrary to the #7534 changelog line "events waiting on other instances are no longer passed over by the next lease". Two causes:

  1. StateManagerRedis._instance_id is chosen once when the manager is built, and the manager is built in the supervisor and inherited by every forked worker (_setup_event_processor re-ids only the token manager). All workers registered in <lock>_waiters under the same member: a second worker's SADD emitted no keyspace sadd event, so the holder's lease was not broken, and any waiter's SREM on exit deregistered every worker. The passed-over waiter only re-registered at its lock_expiration / 10 re-check.
  2. Even with distinct ids, one waiter leaving SREM'd its instance while other waiters of the same instance were still queued.

The instance id is used only as the member of the waiter sets (lock values, leases, pubsub channels and socket records use other ids), so resetting it per worker leaves nothing pointing at the supervisor's id.

Fix

  • App._setup_event_processor calls state_manager._reset_instance_id() next to the token manager's reset; the hook is a no-op on StateManager and picks a new uuid in StateManagerRedis, so the memory and disk managers are untouched.
  • _request_lock_release removes the instance from the waiter set only when its last local waiter for that lock leaves (reusing the existing _lock_waiters list). Its cleanup loops until the registration is stable: it re-registers while a waiter that queued during the SREM is still there, and deregisters again once that waiter has left. The registration runs inside the try, so a waiter cancelled while registering also runs the cleanup.
  • _n_lock_contenders counts only other instances (one atomic script: SCARD minus SISMEMBER(self), still one round trip), so a holder whose only waiters are its own keeps sharing its lease with them.

Test plan

  • New / extended regression tests, failing on unfixed main:
    • tests/units/test_app.py::test_forked_workers_publish_deltas_to_the_socket_owner (extended to a StateManagerRedis): before, AssertionError: assert 'c56438be-…' not in {'c56438be-…'} (worker id equals the supervisor's).
    • tests/units/istate/manager/test_redis.py::test_oplock_contenders_are_the_other_instances_with_a_waiter: before, assert 1 == 0.
    • tests/units/istate/manager/test_redis.py::test_oplock_waiter_leaving_keeps_the_next_lease_from_passing_over_its_instance (lock_expiration 20 s → 2 s re-check; asserts the remaining remote waiter finishes within 1 s): before, TimeoutError (2.003 s on the mock, 2.006 s on real Redis — exactly one re-check interval).
  • Race regression tests for the cleanup loop:
    • tests/units/istate/manager/test_redis.py::test_oplock_waiter_reregistering_while_the_other_waiter_leaves_deregisters
    • tests/units/istate/manager/test_redis.py::test_oplock_waiter_cancelled_while_registering_deregisters
  • test_redis.py 3× on the mock and on a real redis-server; test_state.py, test_app.py, tests/units/istate, test_token_manager.py on the mock, real Redis, and real Redis with REFLEX_OPLOCK_ENABLED=true: pass.
  • Full unit suite: 15636 passed, 66 skipped, coverage 81.43% (the Playwright browser-driver module was excluded locally because no Chromium binary was available). ruff check, ruff format, pyright reflex tests: clean.
  • End to end (prod, Redis, oplock on, fork; 1 client, 300 pipelined events + 100 /api/bump via app.modify_state, 6 runs per cell):
conc 1conc 2conc 4
0.11.0a1, 2 workers2/6 stalled, p99 1058 ms2/6, 1029 ms6/6, 1039 ms
0.11.0a1, 4 workers0/6, 46 ms5/6, 2036 ms6/6, 3035 ms
this branch, 2 workers0/6, 72 ms0/6, 81 ms0/6, 92 ms
this branch, 4 workers0/6, 70 ms0/6, 110 ms0/6, 66 ms

(stalled = an api wait over 500 ms). Final counts exact in 72/72 runs; worker-id probe: 4 pids / 1 id before, 4 / 4 after; spawn with 4 workers also 0/6.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Notes for reviewers

  • Visible behaviour change: a holder whose only waiters are on its own instance now always leases (before it sometimes did single updates); that is the intended oplock behaviour.
  • The module-level debug prefix SMR = f"[SMR:{os.getpid()}]" is still inherited across fork (cosmetic, unchanged).
  • Not addressed here: the instance "instaque" fast path in _wait_lock (byte-identical to 0.10.1) can let a tight local loop starve a remote waiter; separate follow-up.

News fragment: news/+redis-oplock-worker-contenders.bugfix.md.


Generated by Claude Code