test(multidb): e2e failover tests via per-member RESP proxies - #3952
test(multidb): e2e failover tests via per-member RESP proxies#3952ndyakov wants to merge 17 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end test harness and scenario suite for redis.MultiDBClient failover behavior using one client-resp-proxy container per member database (docker-compose profile multidb). This extends the repo’s e2e testing story by validating real failover/fallback semantics under container-level faults (stop/pause) rather than only unit/integration coverage.
Changes:
- Introduce a new env-gated e2e test package (
multidb/e2e) with multiple failover and PubSub scenarios. - Add a small docker-driven harness (
proxyFarm) to inject outages/timeouts by stopping/pausing per-member proxy containers. - Wire up infrastructure to run the suite (
test.multidb.e2eMakefile target +multidbdocker-compose profile/services).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
multidb/e2e/scenarios_test.go |
Adds MultiDB failover/fallback/escalation/manual failover and PubSub-following-active scenarios. |
multidb/e2e/main_test.go |
Env-gates the MultiDB e2e package via TestMain so it only runs when explicitly enabled. |
multidb/e2e/harness_test.go |
Adds docker-driven proxy control + fast options/helpers for stable, quick e2e scenarios. |
Makefile |
Adds test.multidb.e2e target to bring up the multidb profile and run the suite. |
docker-compose.yml |
Adds multidb profile + three per-member proxy services (cae-proxy-db0/1/2). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0871b9a to
42d82a6
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 (7)
multidb/e2e/harness_test.go:91
- RestoreAll ignores errors from
docker start. If the container is missing (or start fails for any other reason), the test will wait up to 30s in awaitListening and then fail with a less-informative timeout. Handledocker starterrors explicitly (at least failing fast on missing containers).
if err := f.docker("unpause", m.Container); err != nil && isMissingContainer(err) {
f.t.Fatalf("proxy container %s does not exist — start the stack with `docker compose --profile multidb up -d`: %v", m.Container, err)
}
_ = f.docker("start", m.Container)
f.awaitListening(i, 30*time.Second)
docker-compose.yml:86
- These MultiDB proxy containers are included in the
allprofile, somake docker.start/make testwill now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for themultidbprofile, dropallhere to avoid extra containers and host port allocations in the default stack.
profiles:
- multidb
- all
docker-compose.yml:104
- These MultiDB proxy containers are included in the
allprofile, somake docker.start/make testwill now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for themultidbprofile, dropallhere to avoid extra containers and host port allocations in the default stack.
profiles:
- multidb
- all
multidb/e2e/scenarios_test.go:196
- publishUntilReceived reads from
msgswithout checking whether the channel is closed. If the PubSub channel closes (e.g. due to an unexpected subscription teardown), the receive case becomes immediately ready and the loop can spin, masking the real failure behind a timeout.
select {
case m := <-msgs:
if m.Payload == tag {
return
}
case <-time.After(250 * time.Millisecond):
}
multidb/e2e/harness_test.go:50
- proxyFarm.docker uses exec.Command without a context/timeout, so a stuck Docker CLI call can hang the entire test process until the global
go test -timeoutfires. Wrap the command in a short CommandContext timeout so failures surface quickly and the suite can't deadlock on a Docker issue.
func (f *proxyFarm) docker(args ...string) error {
out, err := exec.Command("docker", args...).CombinedOutput()
if err != nil {
return fmt.Errorf("docker %v: %v: %s", args, err, out)
}
return nil
}
multidb/e2e/scenarios_test.go:97
- The test comment says the client reports temporary unavailability and then permanent after the attempt budget, but the test currently returns as soon as it sees either Temporary or Permanent. This makes the comment misleading (and the scenario name ambiguous) for readers trying to understand what is actually asserted.
// TestEscalationWhenAllMembersDown: with every member stopped the client
// reports temporary unavailability, then permanent after the attempt budget;
// restarting a member during the temporary phase recovers.
// Spec: test_all_databases_unreachable_error + escalation chain.
docker-compose.yml:68
- These MultiDB proxy containers are included in the
allprofile, somake docker.start/make testwill now start and bind ports for 3 extra proxies even when MultiDB e2e tests are not being run. If they are only needed for themultidbprofile, dropallhere to avoid extra containers and host port allocations in the default stack.
This issue also appears in the following locations of the same file:
- line 84
- line 102
profiles:
- multidb
- all
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
42d82a6 to
41a37c2
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 (5)
docker-compose.yml:86
- Same as cae-proxy-db0: including this service in the
allprofile means it will be started (and bind host ports) for everymake docker.start/make testrun. Unless you explicitly want that behavior, keep it scoped to themultidbprofile only.
profiles:
- multidb
- all
docker-compose.yml:104
- Same as cae-proxy-db0/1: this service being in the
allprofile makes it come up in the default docker stack, which is unnecessary for non-MultiDB test runs and can cause avoidable port/resource usage.
profiles:
- multidb
- all
docker-compose.yml:68
- These MultiDB proxy containers are included in the
allcompose profile. Sincemake docker.startuses--profile all, this will start the MultiDB proxies (and bind ports 17100/18120) during every normal test run, even though the MultiDB e2e suite is env-gated. Keeping these services only under themultidbprofile avoids unnecessary containers/port usage for the default workflow.
This issue also appears in the following locations of the same file:
- line 84
- line 102
profiles:
- multidb
- all
Makefile:130
test.multidb.e2ebrings up compose without exporting the same env vars asdocker.start(e.g.CLIENT_LIBS_TEST_IMAGE,REDIS_VERSION,RE_CLUSTER,RCE_DOCKER). That meansmake test.multidb.e2e CLIENT_LIBS_TEST_IMAGE=...(or other overrides) won’t be honored by docker-compose, and the Redis image/tag can silently differ from the rest of the test workflow.
test.multidb.e2e:
@echo "Running MultiDB e2e tests (per-member proxies)..."
docker compose --profile multidb up -d
@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)
docker compose --profile multidb down
multidb/e2e/scenarios_test.go:196
- If the PubSub channel closes (e.g. due to an underlying connection failure during failover), receiving from
msgsyieldsnilandm.Payloadwill panic. Failing the test with an explicit error here makes failures easier to diagnose and avoids a nil-pointer panic.
select {
case m := <-msgs:
if m.Payload == tag {
return
}
![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: 41a37c2b90
ℹ️ 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".
41a37c2 to
ed85278
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 (6)
docker-compose.yml:90
- Using a mutable
:latesttag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.
cae-proxy-db2:
image: redislabs/client-resp-proxy:latest
container_name: cae-proxy-db2
docker-compose.yml:72
- Using a mutable
:latesttag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.
cae-proxy-db1:
image: redislabs/client-resp-proxy:latest
container_name: cae-proxy-db1
multidb/e2e/harness_test.go:91
- RestoreAll ignores errors from
docker start. If Docker is unavailable or the start fails for another real reason, the test will instead wait up to 30s per member and then fail with the generic "never came back up" message. Treatdocker starterrors as fatal to fail fast with the underlying cause.
if err := f.docker("unpause", m.Container); err != nil && isMissingContainer(err) {
f.t.Fatalf("proxy container %s does not exist — start the stack with `docker compose --profile multidb up -d`: %v", m.Container, err)
}
_ = f.docker("start", m.Container)
f.awaitListening(i, 30*time.Second)
multidb/e2e/scenarios_test.go:196
- In publishUntilReceived, receiving from a closed PubSub channel returns a nil *redis.Message; accessing m.Payload will panic and the failure will be hard to diagnose. Capture the receive ok flag (and optionally nil-check) to fail with a clear error instead of panicking.
select {
case m := <-msgs:
if m.Payload == tag {
return
}
docker-compose.yml:54
- Using a mutable
:latesttag makes the multidb e2e environment non-reproducible and can introduce CI breakages when the proxy image updates. Consider making the image tag configurable via an env var (with a default) so CI/local runs can pin a known-good version.
This issue also appears in the following locations of the same file:
- line 70
- line 88
cae-proxy-db0:
image: redislabs/client-resp-proxy:latest
container_name: cae-proxy-db0
Makefile:128
- test.multidb.e2e runs
docker composewithout exporting the Makefile variables (REDIS_VERSION / CLIENT_LIBS_TEST_IMAGE / etc.). As a result, overrides likemake test.multidb.e2e CLIENT_LIBS_TEST_IMAGE=...won’t affect the compose stack, unlike docker.start/docker.e2e.start which explicitly export these variables.
test.multidb.e2e:
@echo "Running MultiDB e2e tests (per-member proxies)..."
docker compose --profile multidb up -d
@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
ed85278 to
4e06d92
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 1 comment.
Suppressed comments (5)
docker-compose.yml:71
- Using the
:latesttag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.
image: redislabs/client-resp-proxy:latest
docker-compose.yml:89
- Using the
:latesttag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.
image: redislabs/client-resp-proxy:latest
multidb/e2e/harness_test.go:36
- Using
localhosthere makes the tests depend on name resolution (it may prefer::1on IPv6-enabled hosts). Docker port mappings are not always bound on IPv6, so dialinglocalhostcan fail even though127.0.0.1would work. Use an explicit IPv4 loopback address for deterministic connectivity.
members: []memberProxy{
{Container: "cae-proxy-db0", Addr: "localhost:17100"},
{Container: "cae-proxy-db1", Addr: "localhost:17101"},
{Container: "cae-proxy-db2", Addr: "localhost:17102"},
Makefile:128
- Other docker bring-up targets use
--quiet-pull(e.g.docker.startanddocker.e2e.start) to ensure required images are pulled before running tests. Adding it here keeps behavior consistent and avoids failures when the proxy image isn't present locally.
@echo "Running MultiDB e2e tests (per-member proxies)..."
docker compose --profile multidb up -d
@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)
docker-compose.yml:53
- Using the
:latesttag makes the test environment non-reproducible and can introduce unexpected breakage when the image is updated upstream. Consider allowing a pinned override via an environment variable (defaulting to the current value) so CI and local runs can lock to a known-good proxy version.
This issue also appears in the following locations of the same file:
- line 71
- line 89
image: redislabs/client-resp-proxy:latest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e06d92164
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cf55d9505
ℹ️ 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 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (4)
multidb/e2e/scenarios_test.go:294
- Same as the non-pattern PubSub case: if
msgsis closed, the receive yieldsnilandm.Payloadwill panic. Add anilguard so failures report a clear assertion instead of a panic.
case m := <-msgs:
if m.Payload == tag {
return
}
multidb/e2e/scenarios_test.go:246
msgsis a<-chan *redis.Message; if the channel gets closed (e.g. subscription closes due to an unexpected reconnect/teardown), receiving yieldsnilandm.Payloadwill panic. Handle anilmessage and fail the test with a clear error instead of panicking.
This issue also appears on line 291 of the same file.
case m := <-msgs:
if m.Payload == tag {
return
}
Makefile:130
- The proxy readiness loop uses
curl -s, which exits 0 even for HTTP error responses (e.g. 404/500), so the loop can succeed while the API endpoint is unhealthy/miswired. Use-f(and optionally-S) so only a successful HTTP response breaks the loop.
timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || (docker compose --profile multidb down && exit 1); \
.github/workflows/test-e2e.yml:93
- The readiness probe uses
curl -s, which treats HTTP 404/500 as success and can let the job proceed even if/statsis returning an error. Usecurl -f(and-S) so the loop only exits on a healthy HTTP response.
timeout 30 bash -c "until curl -s http://127.0.0.1:$port/stats > /dev/null; do sleep 1; done"
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1e31cedb1
ℹ️ 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 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Makefile:131
test.multidb.e2e's readiness loop relies on the externaltimeoutbinary. Unlikego test -timeout,timeoutisn't available by default on some dev environments (notably macOS), somake test.multidb.e2ecan fail before running any tests. You can implement the same 30s bound port-wait using POSIX shell +date(and also make the curl check fail on HTTP errors via-f).
@for port in 18120 18121 18122; do \
timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \
done
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
docker-compose.yml:86
- Same as cae-proxy-db0: including this service in the
allprofile makes it start during defaultmake docker.start/make testruns, even though MultiDB e2e is meant to be opt-in. Consider keeping it only in themultidbprofile.
profiles:
- multidb
- all
docker-compose.yml:104
- Same as cae-proxy-db0/1: including this service in the
allprofile makes it start during defaultmake docker.start/make testruns, even though MultiDB e2e is meant to be opt-in. Consider keeping it only in themultidbprofile.
profiles:
- multidb
- all
docker-compose.yml:68
- These per-member MultiDB proxy containers are included in the
allprofile, which is whatmake docker.start/make testuses (docker compose --profile all up). That means normal test runs will now always start (and bind ports for) these extra proxies even when the MultiDB e2e suite is not being run. If the intent is for MultiDB E2E to remain opt-in, keep these services only under themultidbprofile.
This issue also appears in the following locations of the same file:
- line 84
- line 102
profiles:
- multidb
- all
Makefile:131
- The readiness probe uses
curl -swhich returns success even for HTTP error status codes (e.g. 404/500). That can let the target be considered "ready" while the proxy API is actually failing, and then the tests fail later with less direct errors. Use-f(and optionally-S) so only a successful HTTP response counts as ready.
@for port in 18120 18121 18122; do \
timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \
.github/workflows/test-e2e.yml:94
- The readiness probe uses
curl -s, which still exits 0 on HTTP error status codes and can treat an unhealthy proxy API as "ready". Use-f(and optionally-S) so only a successful HTTP response satisfies the loop.
for port in 18120 18121 18122; do
timeout 30 bash -c "until curl -s http://127.0.0.1:$port/stats > /dev/null; do sleep 1; done"
done
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Adds the docker-compose multidb profile (three cae-resp-proxy containers, one per member database, fronting the shared standalone Redis) and an env-gated e2e suite that injects faults at the container level: stop for connection-reset outages, pause for hung-connection timeouts. Covers automatic failover under traffic, background-driven failover with zero traffic, auto-fallback to the recovered higher-weight member, the escalation chain, manual failover, and PubSub following the active database. Run with make test.multidb.e2e.
Assert the failover source index, surface publish errors during the PubSub scenario, fail fast when the proxy containers are missing, and add test.multidb.e2e to .PHONY.
- silent gated skip in TestMain (no direct logging) - bounded subscription handshake so a hung proxy fails fast - the escalation scenario polls until ErrPermanentlyNotAvailable instead of stopping at the first temporary error
- docker CLI invocations are bounded by a 30s context so a stuck daemon fails the scenario instead of hanging the suite - the failover scenario verifies the pre-outage value is visible through the new active member before overwriting it
Assert ActiveIndex lands on the dead member immediately after ForceActiveIndex (before traffic can fail it back over), and set ContextTimeoutEnabled on member options so the paused-proxy scenarios are governed by HealthCheckTimeout rather than socket read timeouts.
Require all proxies healthy at startup (InitialDBStateAllAvailable), poll for the OnFailover callback instead of reading it right after the index flips (the announce runs outside the failover lock), and fail — not log — when escalation skips the temporary phase.
Restart a member as soon as temporary unavailability is observed and assert recovery before the terminal error, then drive the permanent escalation separately — a regression that could not recover during the temporary phase would previously still pass.
Only 'is not paused' is benign: a paused container still accepts TCP dials, so a swallowed unpause failure would leave a member frozen while awaitListening reports it healthy.
The suite was env-gated and never executed in CI: bring up the multidb compose profile (three per-member RESP proxies over the standalone Redis), wait for the proxy APIs, and run ./multidb/e2e/ with -race — mirroring the maintnotifications e2e job.
Cover pattern subscriptions across failover, runtime AddDatabase as a real failover target plus RemoveDatabase index shifting under faults, SetWeight steering auto-fallback to the re-weighted member, eight concurrent writers all converging on the new active, all_available refusing construction with a down member, and the active-change and breaker-open callbacks firing on an outage-driven failover.
Use 127.0.0.1 like the repo's other docker-backed e2e tests (localhost can resolve to ::1 first) and remove the unused Unpause wrapper.
A failed eventually() would previously skip the stop signal, leaving workers racing the client teardown in t.Cleanup.
down removes the containers, leaving the diagnostics step nothing to read.
The recovery poll previously ignored every non-nil error, so a regression exhausting the attempt budget during the keep-retrying window would still pass.
Wait for the proxy APIs in the Makefile target like the CI job does, put go test flags before the package path in the workflow for consistency, require the initial failover to land on member 1 in the weight-steering scenario so the later switch proves the reweighting, and give the escalation scenario a larger attempt budget — the strict no-permanent-during-recovery assertion flaked on the legitimate budget-exhaustion boundary when docker start outran 4x500ms.
Tear down the compose stack when 'up' itself fails (a partial start otherwise leaks containers holding the proxy ports), and probe readiness via 127.0.0.1 like the harness dials — IPv6-first hosts resolve localhost to ::1 and can miss Docker's IPv4-published ports.
The teardown ran in a subshell, so its exit 1 only left the subshell: the loop kept probing the remaining ports after the stack was known bad and the recipe could continue into the test run against a stack the teardown had already removed. A brace group exits the recipe shell.
|
@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 (2)
multidb/e2e/harness_test.go:16
- The comment says each member endpoint is a "cae-resp-proxy" container, but the harness actually uses the per-member containers (cae-proxy-db0/1/2). This makes the harness documentation misleading when debugging test failures.
// memberProxy is one MultiDB member endpoint: a cae-resp-proxy container
// fronting the shared target Redis.
Makefile:132
- The Makefile target waits for the proxy /stats endpoints but does not wait for Redis itself to be ready. The CI workflow explicitly pings Redis first; without the Redis readiness check, local runs can start tests while Redis is still booting, increasing flake risk.
@echo "Running MultiDB e2e tests (per-member proxies)..."
docker compose --profile multidb up -d || (docker compose --profile multidb down && exit 1)
@echo "Waiting for the per-member proxies to be ready..."
@for port in 18120 18121 18122; do \
timeout 30 bash -c "until curl -s http://127.0.0.1:$$port/stats > /dev/null; do sleep 1; done" || { docker compose --profile multidb down; exit 1; }; \
done
@E2E_MULTIDB_TESTS=true go test -v -race -count=1 -timeout 15m ./multidb/e2e/... || (docker compose --profile multidb down && exit 1)
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Stacked on the MultiDBClient orchestration PR.
End-to-end failover tests driven by the existing mock RESP proxy
(
redislabs/client-resp-proxy), one container per member database, allfronting the shared standalone Redis (a converged-CRDB approximation). Faults
are injected at the container level:
docker stopfor connection-resetoutages,
docker pausefor hung-connection timeouts.multidb(cae-proxy-db0/1/2, ports 17100-17102)E2E_MULTIDB_TESTS=true),make test.multidb.e2ewith zero traffic, auto-fallback to the recovered higher-weight member,
escalation chain when all members are down (with mid-chain recovery),
manual failover (probe-refused vs forced), and PubSub following the active
database across a member outage
The full suite passes locally in ~21s against live proxies.
Note
Low Risk
Changes are limited to test infrastructure, docker-compose profiles, and CI/Makefile targets; no production client behavior is modified.
Overview
Adds Docker-backed end-to-end coverage for
MultiDBClientusing three per-memberclient-resp-proxycontainers (compose profilemultidb) that all front one Redis, approximating a converged Active-Active topology. Outages are injected withdocker stopanddocker pausevia aproxyFarmharness that dials127.0.0.1to avoid IPv6/localhostflakes.The new
multidb/e2esuite is opt-in (E2E_MULTIDB_TESTS=true, silent no-op otherwise) and exercises failover under traffic, background health-driven failover, auto-fallback by weight, temporary→permanent unavailability when all members are down (with mid-window recovery), manual vs forced active index, Pub/Sub and PSubscribe across failover, runtime add/remove members,SetWeightsteering fallback, concurrent writers, strictInitialDBStateAllAvailableinit, and failover callbacks.CI gains a
test-multidb-e2ejob;make test.multidb.e2emirrors local runs with proxy readiness checks on ports18120–18122and failure-time container logs beforecompose down.Reviewed by Cursor Bugbot for commit b2b370d. Bugbot is set up for automated code reviews on this repo. Configure here.