feat(multidb): add MultiDBClient orchestration layer - #3950
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new MultiDBClient orchestration layer in the root redis package to support client-side active-active failover across multiple underlying Redis databases/clients, and adds a small PubSub API extension to help subscriptions follow active database changes.
Changes:
- Added
MultiDBClient,MultiDBCtrl, configuration/types, and initialization gating viaInitialDBStatepolicies. - Implemented
multidbCorefor command routing, health-check probing, circuit-breaker + detector-driven failover, background failover, and auto-fallback. - Exported
PubSub.Reconnectto force a PubSub re-dial/resubscribe cycle (used by MultiDB PubSub handoff).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
pubsub.go |
Exposes PubSub.Reconnect and applies minor formatting fixes to logger calls. |
multidb.go |
Adds public MultiDB API/types (MultiDBClient, options, errors, control interface) and disables maintnotifications by default for standalone Options members. |
multidb_core.go |
Implements MultiDB routing/failover core, health probing, PubSub-follow-active logic, and background management loop. |
multidb_test.go |
Adds unit tests using hook-faked clients to validate init policies, routing, failover/escalation, membership changes, and concurrency behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
multidb_core.go:570
- SetAutoFallback(enabled) becomes ineffective when AutoFallbackInterval is negative: even after enabling, the background loop checks
c.opts.AutoFallbackInterval > 0and will never run fallback. Either document that negative disables auto-fallback permanently, or treat negative as "disabled by default" by using the absolute value as the interval when enabled.
if c.opts.AutoFallbackInterval > 0 && !c.autoFallbackDisabled.Load() &&
time.Since(lastFallbackCheck) >= c.opts.AutoFallbackInterval {
lastFallbackCheck = time.Now()
c.tryFallbackToPrimary(ctx)
}
multidb_core.go:706
- notifyPubSubs passes through the failover's ctx to PubSub.Reconnect. If failover is triggered from a command with a tight deadline / already-canceled ctx, Reconnect will fail to dial and the PubSub may stay on the old database until a later read error (Reconnect ignores dial errors). Consider using a background context with a bounded timeout for these internal reconnections.
for _, ps := range subs {
ps.Reconnect(ctx, errors.New("multidb: active database changed"))
}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d827179c9b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
multidb.go:412
- MultiDBClient.Process returns the error from core.process but does not set cmd.SetErr(err). In this codebase, Process implementations consistently do this before returning (e.g. Client.Process in redis.go:2146-2149, ClusterClient.Process in osscluster.go:1287-1290, Ring.Process in ring.go:671-675, Conn.Process in redis.go:2454-2457) so cmd.Err()/Result() reflect the latest outcome.
Set cmd.SetErr(err) here to match that contract (this also covers non-retryable errors where core.process just returns).
func (c *MultiDBClient) Process(ctx context.Context, cmd Cmder) error {
return c.core.process(ctx, cmd)
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94a86433e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
multidb_core.go:229
db.cb.IsAllowed()reserves a half-open request slot (internal/circuitbreaker). Calling it during initialization consumes probe budget even though no command is executed, which can prematurely exhaust half-open probes on startup. Use a non-reserving check (e.g.db.selectable()/CheckState()!=Open) when building candidate snapshots.
Allowed: probeHealthy[i] && db.cb.IsAllowed(),
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa40a1c8bd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
aa40a1c to
00af763
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (4)
multidb.go:243
HealthCheckTimeoutis clamped toHealthCheckInterval/2when it is >= the interval, but for very small intervals (e.g. 1ns) the division can produce a zero timeout. That would make probes immediately time out viacontext.WithTimeout(ctx, 0). Consider enforcing a minimum positive timeout after clamping.
if opt.HealthCheckTimeout >= opt.HealthCheckInterval {
opt.HealthCheckTimeout = opt.HealthCheckInterval / 2
}
multidb_core.go:765
- In MultiDB PubSub, the per-connection dial uses the currently active member, but
pubsub.optandpubsub.pushProcessorare only initialized once at creation time. After a failover, the PubSub connection can move to a different underlying client while still using stale protocol/timeouts/push notification processing settings from the original member (e.g.,opt.Protocol, read/write timeouts). Consider refreshingpubsub.opt/pubsub.pushProcessorinsidenewConneach time it dials a member, so the PubSub runtime settings always match the active database it is connecting to.
newConn: func(ctx context.Context, _ string, channels []string) (*pool.Conn, error) {
db, _ := c.activeSnapshot()
if db == nil {
return nil, ErrTemporarilyNotAvailable
}
if db.c == nil {
return nil, errors.New("redis: multidb: PubSub requires a standalone or sentinel active database")
}
cn, err := db.c.pubSubPool.NewConn(ctx, db.c.opt.Network, db.c.opt.Addr, channels)
pubsub.go:188
PubSub.Reconnectis now exported, but it does not return any error and the underlying reconnect path ignores dial/resubscribe errors. The current doc comment reads like it deterministically reconnects and resubscribes, which could be misleading for callers. Consider clarifying that it is best-effort and that failures are surfaced on subsequent PubSub operations.
// Reconnect closes the current connection and re-dials through the PubSub's
// connection factory, resubscribing to all channels and patterns. It is used
// by MultiDBClient to move subscriptions to the new active database after a
// failover, and can be called by applications to force a re-dial.
func (c *PubSub) Reconnect(ctx context.Context, reason error) {
multidb_core.go:869
closeAllholdsdbMuwhile closing each underlying client. Closing clients can block (network I/O, hook cleanup), so keeping the lock held increases the chance of lock contention during shutdown and raises deadlock risk if any close path indirectly tries to acquiredbMu. Consider snapshottingc.dbsunder the lock, clearing it, and closing clients after unlocking.
func (c *multidbCore) closeAll() error {
c.dbMu.Lock()
defer c.dbMu.Unlock()
var firstErr error
for _, db := range c.dbs {
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00af76319d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
00af763 to
0221df0
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0221df0458
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
0221df0 to
8831e50
Compare
- PubSub conns are untracked from their owning member's pool on close (per-conn owner map), the registry deregisters closed subscriptions via onClose, and pubsub options are always initialized (cloned, not shared) - retried commands clear the previous attempt's error before re-execution - SetActiveIndex/ForceActiveIndex read the active index under failoverMu, honor the switch result and reset the failure detector on a real change - RemoveDatabase serializes with failoverMu so the active-index shift cannot race a concurrent failover - cluster member databases also get maintnotifications disabled by default
- Close snapshots the pubsub registry and closes subscriptions outside pubsubMu (PubSub.Close re-enters the registry via onClose) - initial active selection keys off the startup probe results, not just circuit state, so a probe-failed high-weight member is never selected - SetActiveIndex holds failoverMu across lookup, probe and switch so a concurrent RemoveDatabase cannot shift the target - client-side errors (context cancellation, deterministic local rejections per shouldRetry) no longer record breaker/detector failures or trigger failover; they return to the caller directly - failuredetector: replace the claim-then-zero bucket reset with an atomically swapped immutable-epoch bucket state, fixing the lost-count race behind the flaky concurrent-record test
- retryable server replies (LOADING, READONLY, CLUSTERDOWN, ...) now classify as availability failures instead of healthy replies, so the breaker/detector see them and failover can trigger - locally synthesized Redis errors (ErrCrossSlot) are neutral: they no longer count as proof of a healthy server - single-command retries honor Cmder.NoRetry: streaming commands record the failure but are never replayed - candidate snapshots, background checks and failover re-checks use a non-reserving circuit-state read so they cannot exhaust a recovering member's bounded half-open probe budget - auto-fallback selection runs under failoverMu, closing the same removal race fixed for manual failover - a successful SetActiveIndex probe resets the target's breaker so the switch sticks when the member recovered before the grace period
- initialization reconciles breakers with the final probe pass: probe- healthy members get a fresh breaker (a blocking init that opened it is forgiven), probe-failed members get theirs opened so failover cannot select a database already known to be down - process rejects commands with ErrClosed after Close instead of walking the failover escalation - neutral outcomes release the half-open probe slot reserved by IsAllowed - ForceActiveIndex resets the target breaker (unconditional override semantics); SetActiveIndex keeps the probe-then-reset behavior - SetAutoFallback(true) works when the client was constructed with a negative AutoFallbackInterval (interval normalized on the core) - the default failure detector is allocated per client instead of being written back into the caller's MultiDBOptions
- activeSnapshot loads the index under dbMu so it stays coherent with a concurrent RemoveDatabase slice shift - switchActive returns an announce closure that callers invoke after releasing failoverMu, so OnFailover/OnActiveDatabaseChanged callbacks can safely call control APIs without self-deadlocking - the retry loop re-enters the breaker gate after a failover (half-open slot accounting) without consuming a retry attempt - blocking commands with their own read timeout are not retried on local read deadlines, matching *Client semantics - manual selection resets the failure detector also for same-index overrides - AddDatabase opens the breaker of a member that fails its initial probe - the default cluster PING check reports an empty topology as unhealthy - HealthCheckTimeout documents the ContextTimeoutEnabled interaction
Clone the caller's options (init normalizes in place and a second client built from the same value would re-enable retries on the first), reject and re-check Close in AddDatabase so a raced member cannot leak, assign the new member's index before its initial probe, deliver OnCircuitStateChanged asynchronously (FIFO per database) so callbacks may call control APIs without self-deadlocking, stay on a recovered active instead of escalating when no alternate candidate exists, return ErrClosed from post-close PubSub dials so the channel loop terminates, and treat caller-canceled probes as neutral for the breaker.
Check the failure detector before the breaker admission gate so a detector-routed failover cannot leak a half-open probe slot, surface the caller's context error from a canceled pre-failover probe instead of charging a failover attempt, break the consecutive-failed-attempts escalation chain on any successful command, reset the detector after an auto-fallback switch, return terminal ErrClosed for PubSub dials on cluster-only configurations, and stop recording probe outcomes for members removed while a background snapshot was in flight.
Reconnect PubSubs asynchronously after an active-database change so the command that triggered the failover is not billed for dial and resubscribe work, reject RouteByLatency/RouteRandomly on sentinel members during validation (NewFailoverClient panics on them), leave breakers untouched for startup probes that died with the caller's context, wrap default-policy health checks with panic recovery, and document why PubSub options are captured at Subscribe time.
Record background health-check successes through the breaker's external path so they cannot release half-open slots held by real command probes, delegate DBSize/ScriptLoad/ScriptFlush/ScriptExists to the active member so cluster members keep their fan-out semantics, and reject the HIMPORT command family (fieldset registrations are per member client and would silently be lost on failover; fan-out is tracked as follow-up in the design doc).
Classify local pool saturation (ErrPoolTimeout/ErrPoolExhausted) as neutral so client capacity pressure cannot open a healthy member's breaker or trigger failover, and bound consecutive admission-gate rejections: when every selectable member is half-open with a full probe budget, the command now surfaces ErrTemporarilyNotAvailable instead of ping-ponging the active index in a busy loop.
Check the caller's context after successful control-path probes so a SetActiveIndex/AddDatabase whose context died mid-probe cannot still switch the active database or mutate membership, and give sentinel members a private FailoverOptions copy like standalone and cluster members get.
Callbacks run on a library-owned goroutine: recover a panicking user callback (logged via internal.Logger) so it can neither crash the process nor leave the queue wedged in the draining state.
Use the live active index (not the caller's possibly-stale snapshot) for the no-candidate recovery verdict, reset detector and escalation state only when the active switch actually happened (a lost CAS means a concurrent failover already handled it), re-check the caller's context after a successful pre-failover probe and after waiting for the failover lock, and document that a custom FailureDetector must not be shared across clients.
Filter circuit-state callbacks for removed members at dispatch time (a stale probe snapshot can record an outcome after the removal), and re-check the caller's context after acquiring the failover lock in AddDatabase (whose SkipInitialHealthCheck path had no later check) and RemoveDatabase (which never checked it), so operator requests that died while queued cannot still mutate membership.
SetActiveIndex/ForceActiveIndex report ErrClosed after Close instead of an out-of-range error from the drained membership, and an explicit healthy selection clears the failed-failover escalation chain also when the selected member is already active.
Fail construction when the caller's context dies during the startup probes (a late-healthy probe must not birth a live client the caller gave up on — supersedes the earlier neutral-probe reconciliation, now unreachable), re-read the removed flag and index at callback DELIVERY time so a removal landing while a callback is queued cannot surface a reindexed member, clear the failed-failover chain on successful auto-fallback, re-check the context after tryFailover's lock wait, and deep-copy the nested CircuitBreakerConfig into the private options so runtime-added members cannot diverge from the initial ones.
Recover panicking OnFailover/OnActiveDatabaseChanged callbacks (the announce closure also runs on the background loop), reject hand-built HIMPORT commands at Process, add the ErrClosed guard to RemoveDatabase and the cluster fan-out overrides, mark aborted AddDatabase members removed so queued callbacks cannot surface a never-added index, and give the private options copies of HealthChecks and cluster seed addresses. Also pins that mid-read connection resets are recorded on the breaker (shouldRetry matches every net.OpError via Timeout()) — refuting a review claim to the contrary.
Re-check closed after acquiring the failover lock in tryFailover, setActiveIndex and RemoveDatabase (an op queued behind the lock when Close lands must report ErrClosed, not act on drained state), notice Close inside the command retry loop instead of escalating through an empty membership, make canceled probes neutral for BOTH verdicts (a late healthy result could close an open circuit for an operation that returns context.Canceled), and run user failover strategies with panic recovery — they execute on the library-owned background loop too.
A raw Do(ctx, "himport", ...) builds a plain *Cmd that the typed marker interface misses; match the command name as well. Also classify the local HIMPORT rejection as neutral defensively — it is a RedisError only so batch machinery treats it per-command, and must never count as proof of a healthy server.
isRedisReplyError matched only the concrete proto.RedisError string; typed replies parsed by the reader (NOAUTH, NOPERM, EXECABORT, MOVED, ...) fell through to outcomeNeutral instead of outcomeSuccess. Match the root Error marker interface so typed and string replies classify identically (availability replies already reached outcomeFailure via shouldRetry's typed checks).
The single-command gate admitted closed-state commands via IsAllowed, then settled successes with RecordSuccess and neutrals with an unconditional ReleaseHalfOpen — a command outliving a later open -> half-open transition freed a probe slot it never reserved. The gate now uses Allow: unreserved successes count via RecordExternalSuccess and unreserved neutrals release nothing.
DBSize/ScriptLoad/ScriptFlush/ScriptExists delegate to the member client directly when the member is a cluster (fan-out semantics), so MultiDB-level hooks do not wrap them — same contract as Watch; use AddDatabaseHook to instrument that traffic.
4540c2b to
7566dc0
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
export_test.go:145
- These test helpers currently panic with a nil-pointer dereference if an invalid index is passed (dbAt returns nil). It’s better to fail with an explicit panic message so debugging test failures is straightforward.
func (c *MultiDBClient) TestBreakerReserveHalfOpen(index int) bool {
return c.core.dbAt(index).cb.IsAllowed()
}
export_test.go:154
- TestProbeRacingRemoval assumes dbAt(index) always returns a non-nil database. If a test accidentally passes an invalid index, this will panic later with a nil-pointer deref. Adding an explicit check makes failures clearer.
func (c *MultiDBClient) TestProbeRacingRemoval(index int) {
db := c.core.dbAt(index)
if err := c.core.removeDatabase(context.Background(), index); err != nil {
panic(err)
}
export_test.go:170
- TestStaleRecordAfterRemoval assumes dbAt(index) always returns a non-nil database. If a test accidentally passes an invalid index, this will panic with a nil-pointer deref; an explicit check makes test failures clearer.
func (c *MultiDBClient) TestStaleRecordAfterRemoval(index int) {
db := c.core.dbAt(index)
if err := c.core.removeDatabase(context.Background(), index); err != nil {
panic(err)
}
multidb.go:568
- Comment grammar: “cluster members AND the per-shard answers” reads like a typo and is a bit unclear. Reword to make it a proper sentence (e.g., “cluster members aggregate the per-shard answers …”).
// ScriptExists delegates to the active member (cluster members AND the
// per-shard answers so a script only counts as present when every shard has
// it).
export_test.go:138
- These test helpers currently panic with a nil-pointer dereference if an invalid index is passed (dbAt returns nil). It’s better to fail with an explicit panic message so debugging test failures is straightforward.
This issue also appears in the following locations of the same file:
- line 143
- line 150
- line 166
func (c *MultiDBClient) TestBreakerRecordFailure(index int) {
c.core.dbAt(index).cb.RecordFailure()
}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 9 total unresolved issues (including 8 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7566dc0. Configure here.
| } | ||
| c.dbs = append(c.dbs, db) | ||
| c.dbMu.Unlock() | ||
| return idx, nil |
There was a problem hiding this comment.
AddDatabase succeeds when closed
Medium Severity
AddDatabase reads closed once under dbMu and then appends without re-checking. Close sets closed without holding dbMu, so a concurrent Close can mark the client closed between that load and append, yet AddDatabase still returns a new index and nil even though Process and other control APIs already report ErrClosed.
Reviewed by Cursor Bugbot for commit 7566dc0. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7566dc0001
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| err := client.ForEachShard(ctx, func(ctx context.Context, shard *Client) error { | ||
| shards.Add(1) | ||
| return shard.Ping(ctx).Err() |
There was a problem hiding this comment.
When a ClusterClient member uses the built-in default health check (no HealthChecks configured), this calls ForEachShard, which walks replicas as well as masters. In a cluster where a replica is unreachable but the masters are still serving normal MultiDB traffic, startup/background probes mark the whole member unhealthy and can fail initialization or fail over away from a usable cluster; default to master-only probing unless replica routing is explicitly part of the member's traffic.
Useful? React with 👍 / 👎.
Implements the client-side Active-Active failover design (MultiDB v0.9) on
top of the foundation PRs (#3835-#3838):
MultiDBClient(embedscmdable, full command surface) +NewMultiDBClientwithInitialDBStategating (all/majority/one; blockingopt-in via ctx deadline)
MultiDBCtrl: probe-then-switchSetActiveIndex(refuses unhealthytargets), unconditional
ForceActiveIndex, runtimeAddDatabase/RemoveDatabase/SetWeight,SetAutoFallbackfailure detector; retries against the newly selected database
command traffic) and auto-fallback to a recovered higher-weight database
ErrTemporarilyNotAvailable->ErrPermanentlyNotAvailablerate-limited by
FailoverAttemptDelaypublic
MultiDBCircuitBreakerConfig(GracePeriodnaming)exported
PubSub.Reconnect); PubSub errors deliberately do not feed thedetector
RecordMultiDB*recorder pipelineUnit tests use hook-faked clients (no server needed) and run under -race.
Follow-ups tracked separately: redisotel-native geofailover instruments,
cluster-active PubSub, pipeline/autopipeline support (next PR in the stack).
Note
High Risk
Introduces a large, concurrency-heavy failover path on the hot command path (breakers, locks, background health checks, Pub/Sub re-dial) where misclassification or races could mis-route traffic or strand clients in permanent unavailability.
Overview
Adds
MultiDBClient, a drop-in client that routes commands to one active member among standalone, Sentinel, or cluster backends and fails over using per-DB circuit breakers, a pluggable failure detector, health checks, and weight-based (or custom) target selection.Construction and control:
NewMultiDBClientenforcesInitialDBState(all / majority / one healthy) with optional blocking init on a context deadline.MultiDBCtrlexposesSetActiveIndex(probe-first),ForceActiveIndex, runtimeAddDatabase/RemoveDatabase, weights, and auto-fallback toggles. Options are copied on build so sharedMultiDBOptionsare not mutated.Runtime behavior: The command path admits traffic through breakers, classifies outcomes (transport vs app errors vs neutral), retries on other members, and escalates to
ErrTemporarilyNotAvailable/ErrPermanentlyNotAvailable. A background loop probes members, can fail over without command traffic, and can auto-fallback to a higher-weight recovered DB. Pub/Sub follows the active member via a new exportedPubSub.Reconnect. Cluster-active paths delegateDBSizeand script admin to the member client; HIMPORT is rejected (including rawProcess) until fieldsets can fan out.Hardening: Async circuit-state callbacks, panic recovery on strategies/checks/callbacks, races with removal, half-open probe slot accounting, and maintenance notifications disabled by default on members unless configured.
Large hook-based test suite plus
export_testhelpers for breaker and removal races.Reviewed by Cursor Bugbot for commit 7566dc0. Bugbot is set up for automated code reviews on this repo. Configure here.