Threading refactor (7/8): replace shaded-netty HashedWheelTimer with in-process port#7
Open
igorbernstein2 wants to merge 4 commits into
Open
Conversation
mutianf
approved these changes
Jun 29, 2026
igorbernstein2
changed the title
Drops the shaded-netty internal reach-in (HashedWheelTimer) in favor of ScheduledExecutorService. BigtableTimer.newTimeout now takes a caller-supplied Executor so timer callbacks hop off the scheduler thread to the appropriate pool per call site. Benchmark (jetstream100, n=3 per scenario, c32/c100/c500): throughput within 1-2% of the prior NettyWheelTimer impl; CPU/op within 1-3%; p50, p99, p99.9 all within noise.
…imer Drops the JDK ScheduledExecutorService backing in favor of a stripped-down in-process hashed-wheel implementation: single tick thread, 512x10ms wheel (~5s rotation), MPSC pending queue, O(1) insert, O(1) cancel via state CAS with lazy reaping on the bucket's next sweep. Same algorithm and tick geometry as the shaded-netty HashedWheelTimer we used pre-phase-7, but with no Netty dependency. BigtableTimer interface and the per-call Executor contract from c29e are preserved unchanged; only the impl swaps. Benchmark (jetstream100, c100, n=3 medians): HashedWheelTimer: 111,557 ops/s p99 1,527us p9999 94ms ScheduledExecutorTimer: 106,868 ops/s p99 1,592us p9999 174ms Wheel recovers the ~4% throughput regression introduced in c29e and roughly halves the p9999 deep tail.
…ter-ers stop() previously did a plain `if (stopped) return; stopped = true` then snapshot+clear of stopHooks. Three races followed: - Double stop() ran every hook twice (both callers passed the early check before either wrote the flag). - onStop() racing with stop() could land an add after the snapshot/clear, leaving the hook stranded in the set with no worker alive to drive it — the caller's Registration claimed the hook would fire on stop but it never would. - newTimeout() racing with stop() left entries in pendingTimeouts that nobody drained. Switch to AtomicBoolean.compareAndSet for the stop transition. After the CAS, drain stopHooks AND pendingTimeouts in a loop until both observe empty, so any add that started before the CAS but landed after the first drain pass gets caught on the next pass. onStop now invokes the hook synchronously when stop() already happened (ListenableFuture addListener semantics). newTimeout returns a pre-cancelled singleton in the same case so callers see `isCancelled() == true` and skip cleanup paths. The register-then-recheck pattern in each handles the stop()-just-CAS'd-but-not-yet-drained window: the late add either lands in the queue/set (where stop()'s drain loop catches it) or sees the flag and self-removes/self-fires. Drops Scheduled.onStart's try/catch around the timer calls — neither onStop nor newTimeout can throw on the stopped path anymore.
igorbernstein2
force-pushed
the
threading-refactor-phase-6
branch
from
June 30, 2026 17:49
7864221 to
be9a7b3
Compare
igorbernstein2
force-pushed
the
threading-refactor-phase-7
branch
from
June 30, 2026 17:49
62b89be to
39bbfb4
Compare
mutianf
approved these changes
Jun 30, 2026
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
2 participants
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.
Summary
Phase 7 of 8. Drops the only reach-in to gRPC's shaded-netty internals (
HashedWheelTimer). The replacement is a stripped-down in-process hashed-wheel: same algorithm and tick geometry as Netty's, no Netty dependency.What's in here
Two commits, both touching the same
BigtableTimerabstraction:refactor: replace NettyWheelTimer with JDK ScheduledExecutorTimerExecutorparameter onBigtableTimer.newTimeout(so timer callbacks hop off the tick thread to the appropriate pool). Backs the timer with a single-threadScheduledExecutorServiceso the netty dep can be dropped.refactor: replace ScheduledExecutorTimer with in-process HashedWheelTimerBigtableTimerinterface unchanged.Performance
Benchmark (jetstream100, c100, n=3 medians):
The wheel recovers the ~4% throughput regression the JDK scheduler introduced and roughly halves the p9999 deep tail.
Stack position
threading-refactor-phase-6threading-refactor-phase-8— reuse user-configured executor for callbacks