fix: gate federated conversation create on assertFullyConnected - #5430
Open
blackheaven wants to merge 2 commits into
Open
fix: gate federated conversation create on assertFullyConnected#5430blackheaven wants to merge 2 commits into
blackheaven wants to merge 2 commits into
Conversation
Conversation.testAddUnreachableUserFromFederatingBackend[domain=other]
flaked in CI: the conversation create (Test/Conversation.hs) returned HTTP
533 ({unreachable_backends: [<fed2 domain>]}) instead of 201. The 7s test
duration (vs sub-second normally) is consistent with a federation RPC
timeout. The freshly-started dynamic backend C was reachable; only the
always-on static fed2 was reported unreachable -- a transient reachability
blip, not a correctness bug.
Root cause: this test's conversation spans two remote backends concurrently
-- bob on the static fed2 (otherDomain) and charlie on the dynamic backend
C. Conversation creation pings every remote backend concurrently and fails
closed with no retry:
registerRemoteConversationMemberships
-> ensureNoUnreachableBackends =<< runFederatedConcurrentlyEither
(Wire/ConversationSubsystem/Util.hs:833-835)
which throws UnreachableBackends (-> 533, Wire/API/Error/Galley.hs:583-584)
and deletes the half-created conversation (deleteOnUnreachable,
Util.hs:897-905). Creation staying strict is deliberate (WPB-5208 relaxed
reachability only for adding members), so the fix belongs at the test
layer.
Add a reusable readiness gate `assertFullyConnected` in
API.GalleyInternal (polls getFederationStatus until HTTP 200 +
"fully-connected" via `eventually`, which retries on AssertionFailure every
100ms up to Env.timeOutSeconds) and call it before the create.
getFederationStatus pings the same backends and reuses the same reachability
verdict (ensureNoUnreachableBackends) the create will run, and the gate
covers exactly the create's backend set {ownDomain, otherDomain,
cDom.berDomain}. The downstream addMembers 533 assertion (targeting the
stopped dynamic C) is deterministic and unaffected.
A genuine fed2 outage is not masked: if fed2 stays down the gate never
reaches fully-connected within eventually's window and the test fails loudly
at assertFullyConnected (issuing no postConversation, so no create/delete
churn). A residual sub-second TOCTOU race between gate and create remains;
if CI shows it still bites, wrap gate+create together with retryT (not
eventually, since the create can throw a non-AssertionFailure exception).
Runtime trace excerpt:
assertion failure: Actual: 533 / Expected: 201
response body: { unreachable_backends: [ <fed2 domain> ] }
zebot
added
the
ok-to-test
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.
Checklist
changelog.d