From a845a059586492f660bc911cf1ade7ef9928b83e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 13 Feb 2023 18:23:45 +0000 Subject: [PATCH 1/5] Test that Synapse will purge a room during resync --- ...federation_room_join_partial_state_test.go | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/federation_room_join_partial_state_test.go b/tests/federation_room_join_partial_state_test.go index 04fd3c99d..6cbb604f5 100644 --- a/tests/federation_room_join_partial_state_test.go +++ b/tests/federation_room_join_partial_state_test.go @@ -10,6 +10,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/matrix-org/complement/runtime" "io/ioutil" "net" "net/http" @@ -3636,6 +3637,49 @@ func TestPartialStateJoin(t *testing.T) { // - Join+Leave+Rejoin works // - Join + remote kick works // - Join + remote ban works, then cannot rejoin + + t.Run("Purge during resync", func(t *testing.T) { + if runtime.Homeserver != runtime.Synapse { + // TOOD: Pull this into a Synapse-specific suite when someone figures out how + // to do that (https://github.com/matrix-org/complement/issues/226) + t.Skipf("Skipping test of Synapse-internal API on %s", runtime.Homeserver) + } + t.Log("Alice begins a partial join to a room") + alice := deployment.RegisterUser(t, "hs1", "t41alice", "secret", true) + server := createTestServer(t, deployment) + cancel := server.Listen() + defer cancel() + + serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) + psjResult := beginPartialStateJoin(t, server, serverRoom, alice) + defer psjResult.Destroy(t) + + t.Log("Alice waits to see her join") + alice.MustSyncUntil( + t, + client.SyncReq{Filter: buildLazyLoadingSyncFilter(nil)}, + client.SyncJoinedTo(alice.UserID, serverRoom.RoomID), + ) + + t.Log("Alice purges that room") + alice.MustDoFunc(t, "DELETE", []string{"_synapse", "admin", "v1", "rooms", serverRoom.RoomID}, client.WithJSONBody(t, map[string]interface{}{})) + + // Note: clients don't get told about purged rooms. No leave event for you! + t.Log("Alice does an initial sync after the purge") + result, _ := alice.MustSync( + t, + client.SyncReq{Since: "", Filter: buildLazyLoadingSyncFilter(nil)}, + ) + + t.Log("The response should not include the purged room") + must.MatchGJSON( + t, + result, + match.JSONKeyMissing( + fmt.Sprintf("rooms.join.%s", client.GjsonEscape(serverRoom.RoomID)), + ), + ) + }) } // test reception of an event over federation during a resync From 62fd4124edf4a730545e912d15dc60fd5f1a9c1f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 13 Feb 2023 21:01:30 +0000 Subject: [PATCH 2/5] Update tests/federation_room_join_partial_state_test.go Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- tests/federation_room_join_partial_state_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/federation_room_join_partial_state_test.go b/tests/federation_room_join_partial_state_test.go index 6cbb604f5..087e5995b 100644 --- a/tests/federation_room_join_partial_state_test.go +++ b/tests/federation_room_join_partial_state_test.go @@ -3645,7 +3645,7 @@ func TestPartialStateJoin(t *testing.T) { t.Skipf("Skipping test of Synapse-internal API on %s", runtime.Homeserver) } t.Log("Alice begins a partial join to a room") - alice := deployment.RegisterUser(t, "hs1", "t41alice", "secret", true) + alice := deployment.RegisterUser(t, "hs1", "t46alice", "secret", true) server := createTestServer(t, deployment) cancel := server.Listen() defer cancel() From b3482a9a8414e3c9f594c352e81796d10b1fdac5 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 14 Feb 2023 14:19:00 +0000 Subject: [PATCH 3/5] Don't request /members at the end of the test And ensure we don't flake in the style of https://github.com/matrix-org/synapse/issues/13975 --- ...federation_room_join_partial_state_test.go | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/federation_room_join_partial_state_test.go b/tests/federation_room_join_partial_state_test.go index 087e5995b..4bd644c49 100644 --- a/tests/federation_room_join_partial_state_test.go +++ b/tests/federation_room_join_partial_state_test.go @@ -3652,7 +3652,9 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + // NB: because we do not end up joined to this room at the end of the test, + // we do not `defer psjResult.Destroy(t)` as usual; see the comments below + // about races. t.Log("Alice waits to see her join") alice.MustSyncUntil( @@ -3661,6 +3663,17 @@ func TestPartialStateJoin(t *testing.T) { client.SyncJoinedTo(alice.UserID, serverRoom.RoomID), ) + // Synapse's partial-state-resync process can race with the purge. + // If the purge completes and Synapse then makes a /state_ids request to us + // after we've shut down the complement test server, we can end up with flakey + // test failures (c.f. https://github.com/matrix-org/synapse/issues/13975) + // Avoid this by + // - waiting for Synapse to make a state_ids request + // - serving the response after the purge (see next comment). + + t.Log("Wait for /state_ids request") + psjResult.AwaitStateIdsRequest(t) + t.Log("Alice purges that room") alice.MustDoFunc(t, "DELETE", []string{"_synapse", "admin", "v1", "rooms", serverRoom.RoomID}, client.WithJSONBody(t, map[string]interface{}{})) @@ -3671,6 +3684,12 @@ func TestPartialStateJoin(t *testing.T) { client.SyncReq{Since: "", Filter: buildLazyLoadingSyncFilter(nil)}, ) + // Send the state ids response now. Synapse will try to process it and fail + // because of the purge. There are no other destinations here so Synapse should + // give up the resync process and not make any more requests to the complement + // HS, avoiding the flake described above. + psjResult.FinishStateRequest() + t.Log("The response should not include the purged room") must.MatchGJSON( t, From fd2f403df669761238dd59fa4751f4e6a35ed011 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 14 Feb 2023 16:42:56 +0000 Subject: [PATCH 4/5] Repeat last /sync query to cope with worker races --- ...federation_room_join_partial_state_test.go | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/tests/federation_room_join_partial_state_test.go b/tests/federation_room_join_partial_state_test.go index 4bd644c49..a3cc43bba 100644 --- a/tests/federation_room_join_partial_state_test.go +++ b/tests/federation_room_join_partial_state_test.go @@ -3678,10 +3678,27 @@ func TestPartialStateJoin(t *testing.T) { alice.MustDoFunc(t, "DELETE", []string{"_synapse", "admin", "v1", "rooms", serverRoom.RoomID}, client.WithJSONBody(t, map[string]interface{}{})) // Note: clients don't get told about purged rooms. No leave event for you! - t.Log("Alice does an initial sync after the purge") - result, _ := alice.MustSync( + t.Log("Alice does an initial sync after the purge, until the response does not include the purged room") + + // Note: we retry this sync a few times, because the purge may happen on another + // worker to that serving the sync response. + queryParams := url.Values{ + "timeout": []string{"1000"}, + "filter": []string{buildLazyLoadingSyncFilter(nil)}, + } + matcher := match.JSONKeyMissing( + fmt.Sprintf("rooms.join.%s", client.GjsonEscape(serverRoom.RoomID)), + ) + alice.MustDoFunc( t, - client.SyncReq{Since: "", Filter: buildLazyLoadingSyncFilter(nil)}, + "GET", + []string{"_matrix", "client", "v3", "sync"}, + client.WithQueries(queryParams), + client.WithRetryUntil(5*time.Second, func(res *http.Response) bool { + body := client.ParseJSON(t, res) + err := matcher(body) + return err == nil + }), ) // Send the state ids response now. Synapse will try to process it and fail @@ -3689,15 +3706,6 @@ func TestPartialStateJoin(t *testing.T) { // give up the resync process and not make any more requests to the complement // HS, avoiding the flake described above. psjResult.FinishStateRequest() - - t.Log("The response should not include the purged room") - must.MatchGJSON( - t, - result, - match.JSONKeyMissing( - fmt.Sprintf("rooms.join.%s", client.GjsonEscape(serverRoom.RoomID)), - ), - ) }) } From e982bbea339837b1db03528cd454d3b2d6267e0c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 14 Feb 2023 16:43:18 +0000 Subject: [PATCH 5/5] Ignore leave events sent to us after purge --- tests/federation_room_join_partial_state_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/federation_room_join_partial_state_test.go b/tests/federation_room_join_partial_state_test.go index a3cc43bba..ff930e2e2 100644 --- a/tests/federation_room_join_partial_state_test.go +++ b/tests/federation_room_join_partial_state_test.go @@ -3646,7 +3646,12 @@ func TestPartialStateJoin(t *testing.T) { } t.Log("Alice begins a partial join to a room") alice := deployment.RegisterUser(t, "hs1", "t46alice", "secret", true) - server := createTestServer(t, deployment) + // Ignore PDUs (leaves from shutting down the room) and EDUs (presence). + server := createTestServer( + t, + deployment, + federation.HandleTransactionRequests(nil, nil), + ) cancel := server.Listen() defer cancel()