mirror of
https://github.com/element-hq/synapse.git
synced 2026-07-19 18:07:07 +00:00
Fix Complement test flake when restarting Synapse workers (cross-test pollution caused by nginx upstreams being temporarily unavailable) (#19936)
Fix https://github.com/element-hq/synapse/issues/19907 The flake manifested as a failure pointing at `TestMessagesOverFederation/Backfill_from_nearby_backward_extremities_past_token` but was actually caused by some cross-test pollution from an earlier test (`TestOIDCProviderUnavailable`) causing some workers to be temporarily unavailable. As explained in https://github.com/element-hq/synapse/issues/19907#issuecomment-4917362461, > ### Cross-test pollution > > When I point an LLM at the logs, it points to `TestOIDCProviderUnavailable` being the culprit because of the server restarts polluting this test. When this flake happens, we can indeed see that `TestOIDCProviderUnavailable` runs before `TestMessagesOverFederation`. > > ``` > PASS TestEventBetweenMakeJoinAndSendJoinIsNotLost 15.08s > PASS TestFederation/parallel/HS2_->_HS1 1.5s > PASS TestFederation/parallel/HS1_->_HS2 1.51s > PASS TestFederation/parallel 0s > PASS TestFederation 15.12s > PASS TestOIDCProviderUnavailable//login/sso/redirect_shows_HTML_error 0.02s > PASS TestOIDCProviderUnavailable 8.62s > FAIL TestMessagesOverFederation/Backfill_from_nearby_backward_extremities_past_token 0.89s > FAIL TestMessagesOverFederation 1.1s > PASS TestSynapseVersion/Synapse_version_matches_current_git_checkout 0.97s > PASS TestSynapseVersion 0.97s > ``` > > The pollution happens because we enable [`COMPLEMENT_ENABLE_DIRTY_RUNS`](https://github.com/element-hq/synapse/blob/c63d77a79d7157f26f849684520ba9e99f4d07c0/scripts-dev/complement.sh#L309-L311) ([docs](https://github.com/matrix-org/complement/blob/0e6f8552ff0c99fddb97222399efed3e1f0cb91a/ENVIRONMENT.md#complement_enable_dirty_runs)) which means Complement will reuse deployments (shares homeservers between tests). > > During the `TestOIDCProviderUnavailable` test, there are some stray federation requests that hit the homeserver while it's still booting which marks the nginx upstream as unavailable for 10 seconds. nginx has a default of [`max_fails=1`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#max_fails) and [`fail_timeout=10s`](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#fail_timeout). Then when `TestMessagesOverFederation` starts, we're still in the 10 second unavailable window and nginx doesn't even try to connect at all. > > <details> > <summary>LLM summary of the logs and how this happens in practice</summary> > > 1. **19:52:10–11** — the previous federation test finishes: `user-3:hs2` does a faster-join (`send_join?omit_members=true`) to `!YnyCRpCLIpimppIreR:hs1`, so `hs2` kicks off a `sync_partial_state_room` background resync that is still running when the test ends. > 2. **19:52:11.6** — `TestOIDCProviderUnavailable` starts and calls `deployment.StopServer(t, "hs1")` / `StartServer` (`complement/tests/oidc_test.go:78-80`) to apply an OIDC config fragment. `hs1` gets `SIGTERM`; new supervisord at 19:52:13.3. `hs2` is not restarted and keeps retrying its unfinished work. > 3. **19:52:14** — `hs1`'s nginx is up, but the Synapse workers aren't: `federation_inbound` only listens on `18015` at 19:52:18.7, `federation_reader` on `18016` at 19:52:19.0. > 4. **19:52:15–16** — `hs2`'s retries arrive in that gap: `PUT /_matrix/federation/v1/send/…` → `18015` refused; `GET /state_ids/!YnyCRpCLIpimppIreR:hs1` → `18016` refused. With nginx defaults (`max_fails=1`, `fail_timeout=10s`) and only one server per upstream block, both upstreams are now marked down until ~19:52:25/26. (Side casualty: `hs2`'s partial-state resync gives up — "We can't get valid state history" — and puts `hs1` on a 10-minute federation backoff.) > 5. **19:52:21.5** — the failing test's `make_join` for `@user-5-bob:hs2` reaches `hs1`'s nginx → `no live upstreams` → `502` → the join fails, even though the worker has been listening for 2.5 seconds by then. > 6. **19:52:22** — `TestSynapseVersion`'s `GET /_matrix/federation/v1/version` hits the same dead upstream → `502` → it fails too. > > So it's a flake caused by a race between the OIDC test's container restart, hs2's background federation retries, and nginx's passive health-check — not anything wrong with the backfill logic under test. > > </details> > > > We can indeed confirm this suspicion with these logs > > ❌ https://github.com/element-hq/synapse/actions/runs/28888070856/job/85701936736 (archive: [85701936736.log](https://github.com/user-attachments/files/29809986/85701936736.log)): > ``` > Error: 2026/07/07 19:52:20 [error] 34#34: *1 no live upstreams while connecting to upstream, client: 172.18.0.3, server: localhost, request: "PUT /_matrix/federation/v1/send/1783453927718 HTTP/1.1", upstream: "http://federation_inbound/_matrix/federation/v1/send/1783453927718", host: "hs1" > ... > > Error: 2026/07/07 19:52:21 [error] 33#33: *4 no live upstreams while connecting to upstream, client: 172.18.0.3, server: localhost, request: "GET /_matrix/federation/v1/make_join/%21scAbyTQDdaauGYicpU%3Ahs1/%40user-5-bob%3Ahs2?ver=1&ver=2&ver=3&ver=4&ver=5&ver=6&ver=7&ver=8&ver=9&ver=10&ver=11&ver=12&ver=org.matrix.msc3757.10&ver=org.matrix.msc3757.11&ver=org.matrix.hydra.11 HTTP/1.1", upstream: "http://federation_reader/_matrix/federation/v1/make_join/%21scAbyTQDdaauGYicpU%3Ahs1/%40user-5-bob%3Ahs2?ver=1&ver=2&ver=3&ver=4&ver=5&ver=6&ver=7&ver=8&ver=9&ver=10&ver=11&ver=12&ver=org.matrix.msc3757.10&ver=org.matrix.msc3757.11&ver=org.matrix.hydra.11", host: "hs1" > ``` > > The fix here would be to disable nginx's unavailable upstream behavior by configuring `max_fails=0` in the upstream block: https://github.com/element-hq/synapse/blob/c63d77a79d7157f26f849684520ba9e99f4d07c0/docker/configure_workers_and_start.py#L374-L378
This commit is contained in:
@@ -0,0 +1 @@
|
||||
Fix Complement test flake when restarting Synapse workers (cross-test pollution caused by nginx upstreams being temporarily unavailable).
|
||||
@@ -1070,14 +1070,21 @@ def generate_worker_files(
|
||||
# Determine the load-balancing upstreams to configure
|
||||
nginx_upstream_config = ""
|
||||
for upstream_worker_base_name, upstream_worker_ports in nginx_upstreams.items():
|
||||
# We use `max_fails=0` to prevent nginx from marking an upstream as unavailable
|
||||
# after it fails to contact the Synapse worker. Otherwise, if nginx sees a
|
||||
# Synapse worker as unavailable once, it will be marked as unavailable for 10
|
||||
# seconds (`fail_timeout` default).
|
||||
#
|
||||
# This is necessary because we use `COMPLEMENT_ENABLE_DIRTY_RUNS` (re-uses
|
||||
# deployments/homeservers) and we don't want any cross-test pollution from
|
||||
# stopping/starting homeservers.
|
||||
body = ""
|
||||
if using_unix_sockets:
|
||||
for port in upstream_worker_ports:
|
||||
body += f" server unix:/run/worker.{port};\n"
|
||||
|
||||
body += f" server unix:/run/worker.{port} max_fails=0;\n"
|
||||
else:
|
||||
for port in upstream_worker_ports:
|
||||
body += f" server localhost:{port};\n"
|
||||
body += f" server localhost:{port} max_fails=0;\n"
|
||||
|
||||
# Add to the list of configured upstreams
|
||||
nginx_upstream_config += NGINX_UPSTREAM_CONFIG_BLOCK.format(
|
||||
|
||||
Reference in New Issue
Block a user