temporalio / temporalio/s2s-proxy
Flaky TestMultiClientUpdateStateAsyncClient hangs to 5m timeout (require in goroutine deadlocks on close race)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 16
- Forks
- 9
- Avg merge
- 6d 23h
- Merged PRs (30d)
- 12
Description
Summary
TestMultiClientUpdateStateAsyncClient (in proxy/test/multi_client_conn_test.go) is flaky: on a lost race it hangs until the package-wide -timeout=5m fires and the whole proxy/test binary panics, instead of failing cleanly. A flaky failure is being reported as a 5-minute CI timeout.
panic: test timed out after 5m0s
running tests:
TestMultiClientUpdateStateAsyncClient (4m36s)
...
FAIL github.com/temporalio/s2s-proxy/proxy/test 300.224s
make: *** [Makefile:96: test] Error 1
The Failed to get replication tasks log lines are a red herring
Failed to get replication tasks ... "xdc-source-cluster": "cluster-b-raaarmod" ... "error": "connection error: ..."
These come from leaked replication-task-processor goroutines of earlier integration tests in the same proxy/test binary (the failover/replication suites start real clusters; after teardown their background workers keep retrying against the torn-down cluster-b-…). TestMultiClientUpdateStateAsyncClient is a yamux/gRPC unit test with no cluster-b, so these lines only pollute the log — they are not the cause.
Root cause: deadlock on <-responsesCh
The worker goroutine only sends its result after all 1000 DescribeCluster calls complete:
go func() {
responses := make(map[string]int, 10)
for range 1000 {
_ = wedge.Acquire(context.Background(), 1)
wg.Done()
resp, err := client.DescribeCluster(context.Background(), &adminservice.DescribeClusterRequest{})
require.NoError(t, err) // <-- called from a NON-test goroutine
responses[resp.ClusterName]++
}
responsesCh <- responses // <-- only reached if the loop finishes
}()
...
responses := <-responsesCh // <-- test goroutine blocks here forever
require.NoError(t, err) is invoked from a spawned goroutine. On error it calls t.FailNow() → runtime.Goexit(), which terminates only that goroutine — it never reaches responsesCh <- responses. The test goroutine then blocks on <-responsesCh indefinitely, so the package hits -timeout=5m and panics, pointing at whichever test is running (this one). A clean assertion failure is turned into a 5-minute hang (the classic "testify require in a non-test goroutine" pitfall).
Why it's flaky
After UpdateState({mux1,mux2,mux3}), then CloseMux(0) + CloseMux(2), the test asserts round_robin sends 0 requests to the dead mux 2 (responses["adminService on mux 2"] == 0). There is a race between CloseMux(2) and the round_robin LB marking that subconn down. If a request is dispatched to mux 2 in that window, DescribeCluster returns a connection error → require.NoError → Goexit → deadlock → 5-minute timeout. When the LB wins the race, the test passes.
Suggested fixes
- Make failures fail cleanly (removes the hang): do not use
require/FailNowinside the worker goroutine. Capture the error and surface it to the test goroutine (e.g. senderrover a channel andselecton result-or-error), then assert in the test body. A bad route then produces an immediate, legible failure instead of a 5-minute timeout. - Address the underlying flake: if the LB should never dispatch to a just-closed mux, wait for the connection-state update to propagate before releasing the second batch (
wedge.Release(500)), or tolerate/retryUnavailablefor the closed subconn. If routing-to-closed is expected to error, themux 2 == 0assertion needs rethinking. - Reduce cross-test log bleed: the leaked replication-task goroutines indicate earlier suites do not fully stop their workers on teardown; cleaner teardown (or isolating these unit tests in a separate package) would keep failure logs readable.
Environment
- Package:
github.com/temporalio/s2s-proxy/proxy/test - Test:
TestMultiClientUpdateStateAsyncClient grpc@v1.80.0, test timeout-timeout=5m(MakefileTEST_ARG)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in proxy/test/multi_client_conn_test.go at TestMultiClientUpdateStateAsyncClient, especially the worker goroutine's require.NoError call and responsesCh receive. Run the targeted test and inspect the close/state-update sequence around CloseMux and wedge.Release. Done means a routing error fails cleanly in the test goroutine instead of hanging until the five-minute timeout, with the flaky closed-mux behavior addressed or explicitly handled.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100