From 872788fd1676987c1320977dc566886e6280d8dd Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 30 Jun 2021 11:27:34 +0100 Subject: [PATCH 1/3] Change how servers are selected for missing auth/prev events --- .../personalities/federationapi.go | 2 +- federationapi/federationapi.go | 2 + federationapi/federationapi_test.go | 2 +- federationapi/routing/routing.go | 3 +- federationapi/routing/send.go | 58 +++++++++++-------- setup/monolith.go | 2 +- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/cmd/dendrite-polylith-multi/personalities/federationapi.go b/cmd/dendrite-polylith-multi/personalities/federationapi.go index 498be3c437..5ff085282c 100644 --- a/cmd/dendrite-polylith-multi/personalities/federationapi.go +++ b/cmd/dendrite-polylith-multi/personalities/federationapi.go @@ -33,7 +33,7 @@ func FederationAPI(base *setup.BaseDendrite, cfg *config.Dendrite) { base.PublicFederationAPIMux, base.PublicKeyAPIMux, &base.Cfg.FederationAPI, userAPI, federation, keyRing, rsAPI, fsAPI, base.EDUServerClient(), keyAPI, - &base.Cfg.MSCs, + &base.Cfg.MSCs, nil, ) base.SetupAndServeHTTP( diff --git a/federationapi/federationapi.go b/federationapi/federationapi.go index 6188b283ee..436f507a85 100644 --- a/federationapi/federationapi.go +++ b/federationapi/federationapi.go @@ -39,10 +39,12 @@ func AddPublicRoutes( eduAPI eduserverAPI.EDUServerInputAPI, keyAPI keyserverAPI.KeyInternalAPI, mscCfg *config.MSCs, + servers routing.ServersInRoomProvider, ) { routing.Setup( fedRouter, keyRouter, cfg, rsAPI, eduAPI, federationSenderAPI, keyRing, federation, userAPI, keyAPI, mscCfg, + servers, ) } diff --git a/federationapi/federationapi_test.go b/federationapi/federationapi_test.go index b97876d3df..505a11dae0 100644 --- a/federationapi/federationapi_test.go +++ b/federationapi/federationapi_test.go @@ -31,7 +31,7 @@ func TestRoomsV3URLEscapeDoNot404(t *testing.T) { fsAPI := base.FederationSenderHTTPClient() // TODO: This is pretty fragile, as if anything calls anything on these nils this test will break. // Unfortunately, it makes little sense to instantiate these dependencies when we just want to test routing. - federationapi.AddPublicRoutes(base.PublicFederationAPIMux, base.PublicKeyAPIMux, &cfg.FederationAPI, nil, nil, keyRing, nil, fsAPI, nil, nil, &cfg.MSCs) + federationapi.AddPublicRoutes(base.PublicFederationAPIMux, base.PublicKeyAPIMux, &cfg.FederationAPI, nil, nil, keyRing, nil, fsAPI, nil, nil, &cfg.MSCs, nil) baseURL, cancel := test.ListenAndServe(t, base.PublicFederationAPIMux, true) defer cancel() serverName := gomatrixserverlib.ServerName(strings.TrimPrefix(baseURL, "https://")) diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index 07a28c3fc3..b094003a9c 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -50,6 +50,7 @@ func Setup( userAPI userapi.UserInternalAPI, keyAPI keyserverAPI.KeyInternalAPI, mscCfg *config.MSCs, + servers ServersInRoomProvider, ) { v2keysmux := keyMux.PathPrefix("/v2").Subrouter() v1fedmux := fedMux.PathPrefix("/v1").Subrouter() @@ -99,7 +100,7 @@ func Setup( func(httpReq *http.Request, request *gomatrixserverlib.FederationRequest, vars map[string]string) util.JSONResponse { return Send( httpReq, request, gomatrixserverlib.TransactionID(vars["txnID"]), - cfg, rsAPI, eduAPI, keyAPI, keys, federation, mu, + cfg, rsAPI, eduAPI, keyAPI, keys, federation, mu, servers, ) }, )).Methods(http.MethodPut, http.MethodOptions) diff --git a/federationapi/routing/send.go b/federationapi/routing/send.go index 032c0c3b4c..73ae9a7675 100644 --- a/federationapi/routing/send.go +++ b/federationapi/routing/send.go @@ -101,6 +101,7 @@ func Send( keys gomatrixserverlib.JSONVerifier, federation *gomatrixserverlib.FederationClient, mu *internal.MutexByRoom, + servers ServersInRoomProvider, ) util.JSONResponse { t := txnReq{ rsAPI: rsAPI, @@ -109,6 +110,7 @@ func Send( federation: federation, hadEvents: make(map[string]bool), haveEvents: make(map[string]*gomatrixserverlib.HeaderedEvent), + servers: servers, keyAPI: keyAPI, roomsMu: mu, } @@ -158,16 +160,20 @@ func Send( } } +type ServersInRoomProvider interface { + GetServersForRoom(ctx context.Context, roomID string, event *gomatrixserverlib.Event) []gomatrixserverlib.ServerName +} + type txnReq struct { gomatrixserverlib.Transaction - rsAPI api.RoomserverInternalAPI - eduAPI eduserverAPI.EDUServerInputAPI - keyAPI keyapi.KeyInternalAPI - keys gomatrixserverlib.JSONVerifier - federation txnFederationClient - servers []gomatrixserverlib.ServerName - serversMutex sync.RWMutex - roomsMu *internal.MutexByRoom + rsAPI api.RoomserverInternalAPI + eduAPI eduserverAPI.EDUServerInputAPI + keyAPI keyapi.KeyInternalAPI + keys gomatrixserverlib.JSONVerifier + federation txnFederationClient + roomsMu *internal.MutexByRoom + // something that can tell us about which servers are in a room right now + servers ServersInRoomProvider // a list of events from the auth and prev events which we already had hadEvents map[string]bool // local cache of events for auth checks, etc - this may include events @@ -466,22 +472,24 @@ func (t *txnReq) processDeviceListUpdate(ctx context.Context, e gomatrixserverli } } -func (t *txnReq) getServers(ctx context.Context, roomID string) []gomatrixserverlib.ServerName { - t.serversMutex.Lock() - defer t.serversMutex.Unlock() +func (t *txnReq) getServers(ctx context.Context, roomID string, event *gomatrixserverlib.Event) []gomatrixserverlib.ServerName { + // The server that sent us the event should be sufficient to tell us about missing + // prev and auth events. + servers := []gomatrixserverlib.ServerName{t.Origin} + // If a specific room-to-server provider exists then use that. This will primarily + // be used for the P2P demos. if t.servers != nil { - return t.servers - } - t.servers = []gomatrixserverlib.ServerName{t.Origin} - serverReq := &api.QueryServerJoinedToRoomRequest{ - RoomID: roomID, - } - serverRes := &api.QueryServerJoinedToRoomResponse{} - if err := t.rsAPI.QueryServerJoinedToRoom(ctx, serverReq, serverRes); err == nil { - t.servers = append(t.servers, serverRes.ServerNames...) - util.GetLogger(ctx).Infof("Found %d server(s) to query for missing events in %q", len(t.servers), roomID) + return append(servers, t.servers.GetServersForRoom(ctx, roomID, event)...) + } + // If the event origin is different to the transaction origin then we can use + // this as a last resort. The origin server that created the event would have + // had to know the auth and prev events. + if event != nil { + if origin := event.Origin(); origin != t.Origin { + servers = append(servers, origin) + } } - return t.servers + return servers } func (t *txnReq) processEvent(ctx context.Context, e *gomatrixserverlib.Event) error { @@ -566,7 +574,7 @@ func (t *txnReq) retrieveMissingAuthEvents( withNextEvent: for missingAuthEventID := range missingAuthEvents { withNextServer: - for _, server := range t.getServers(ctx, e.RoomID()) { + for _, server := range t.getServers(ctx, e.RoomID(), e) { logger.Infof("Retrieving missing auth event %q from %q", missingAuthEventID, server) tx, err := t.federation.GetEvent(ctx, server, missingAuthEventID) if err != nil { @@ -948,7 +956,7 @@ func (t *txnReq) getMissingEvents(ctx context.Context, e *gomatrixserverlib.Even } var missingResp *gomatrixserverlib.RespMissingEvents - servers := t.getServers(ctx, e.RoomID()) + servers := t.getServers(ctx, e.RoomID(), e) for _, server := range servers { var m gomatrixserverlib.RespMissingEvents if m, err = t.federation.LookupMissingEvents(ctx, server, e.RoomID(), gomatrixserverlib.MissingEvents{ @@ -1220,7 +1228,7 @@ func (t *txnReq) lookupEvent(ctx context.Context, roomVersion gomatrixserverlib. } var event *gomatrixserverlib.Event found := false - servers := t.getServers(ctx, roomID) + servers := t.getServers(ctx, roomID, nil) for _, serverName := range servers { txn, err := t.federation.GetEvent(ctx, serverName, missingEventID) if err != nil || len(txn.PDUs) == 0 { diff --git a/setup/monolith.go b/setup/monolith.go index a740ebb7f6..235be44747 100644 --- a/setup/monolith.go +++ b/setup/monolith.go @@ -68,7 +68,7 @@ func (m *Monolith) AddAllPublicRoutes(process *process.ProcessContext, csMux, ss federationapi.AddPublicRoutes( ssMux, keyMux, &m.Config.FederationAPI, m.UserAPI, m.FedClient, m.KeyRing, m.RoomserverAPI, m.FederationSenderAPI, - m.EDUInternalAPI, m.KeyAPI, &m.Config.MSCs, + m.EDUInternalAPI, m.KeyAPI, &m.Config.MSCs, nil, ) mediaapi.AddPublicRoutes(mediaMux, &m.Config.MediaAPI, m.UserAPI, m.Client) syncapi.AddPublicRoutes( From a0e92750311e8ed0b058b7e7b1ac99bcbb09768b Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 30 Jun 2021 11:29:23 +0100 Subject: [PATCH 2/3] Shuffle order --- federationapi/routing/send.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/federationapi/routing/send.go b/federationapi/routing/send.go index 73ae9a7675..436f07e3b9 100644 --- a/federationapi/routing/send.go +++ b/federationapi/routing/send.go @@ -476,11 +476,6 @@ func (t *txnReq) getServers(ctx context.Context, roomID string, event *gomatrixs // The server that sent us the event should be sufficient to tell us about missing // prev and auth events. servers := []gomatrixserverlib.ServerName{t.Origin} - // If a specific room-to-server provider exists then use that. This will primarily - // be used for the P2P demos. - if t.servers != nil { - return append(servers, t.servers.GetServersForRoom(ctx, roomID, event)...) - } // If the event origin is different to the transaction origin then we can use // this as a last resort. The origin server that created the event would have // had to know the auth and prev events. @@ -489,6 +484,11 @@ func (t *txnReq) getServers(ctx context.Context, roomID string, event *gomatrixs servers = append(servers, origin) } } + // If a specific room-to-server provider exists then use that. This will primarily + // be used for the P2P demos. + if t.servers != nil { + servers = append(servers, t.servers.GetServersForRoom(ctx, roomID, event)...) + } return servers } From edf8b96e11cbd34c10359ad3f78f8b40d3de5db2 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Wed, 30 Jun 2021 11:38:20 +0100 Subject: [PATCH 3/3] Move ServersInRoomProvider into api package --- federationapi/api/servers.go | 11 +++++++++++ federationapi/federationapi.go | 3 ++- federationapi/routing/routing.go | 3 ++- federationapi/routing/send.go | 9 +++------ 4 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 federationapi/api/servers.go diff --git a/federationapi/api/servers.go b/federationapi/api/servers.go new file mode 100644 index 0000000000..6bb15763d5 --- /dev/null +++ b/federationapi/api/servers.go @@ -0,0 +1,11 @@ +package api + +import ( + "context" + + "github.com/matrix-org/gomatrixserverlib" +) + +type ServersInRoomProvider interface { + GetServersForRoom(ctx context.Context, roomID string, event *gomatrixserverlib.Event) []gomatrixserverlib.ServerName +} diff --git a/federationapi/federationapi.go b/federationapi/federationapi.go index 436f507a85..b3297434a2 100644 --- a/federationapi/federationapi.go +++ b/federationapi/federationapi.go @@ -17,6 +17,7 @@ package federationapi import ( "github.com/gorilla/mux" eduserverAPI "github.com/matrix-org/dendrite/eduserver/api" + federationAPI "github.com/matrix-org/dendrite/federationapi/api" federationSenderAPI "github.com/matrix-org/dendrite/federationsender/api" keyserverAPI "github.com/matrix-org/dendrite/keyserver/api" roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" @@ -39,7 +40,7 @@ func AddPublicRoutes( eduAPI eduserverAPI.EDUServerInputAPI, keyAPI keyserverAPI.KeyInternalAPI, mscCfg *config.MSCs, - servers routing.ServersInRoomProvider, + servers federationAPI.ServersInRoomProvider, ) { routing.Setup( fedRouter, keyRouter, cfg, rsAPI, diff --git a/federationapi/routing/routing.go b/federationapi/routing/routing.go index b094003a9c..8f33c7660c 100644 --- a/federationapi/routing/routing.go +++ b/federationapi/routing/routing.go @@ -20,6 +20,7 @@ import ( "github.com/gorilla/mux" "github.com/matrix-org/dendrite/clientapi/jsonerror" eduserverAPI "github.com/matrix-org/dendrite/eduserver/api" + federationAPI "github.com/matrix-org/dendrite/federationapi/api" federationSenderAPI "github.com/matrix-org/dendrite/federationsender/api" "github.com/matrix-org/dendrite/internal" "github.com/matrix-org/dendrite/internal/httputil" @@ -50,7 +51,7 @@ func Setup( userAPI userapi.UserInternalAPI, keyAPI keyserverAPI.KeyInternalAPI, mscCfg *config.MSCs, - servers ServersInRoomProvider, + servers federationAPI.ServersInRoomProvider, ) { v2keysmux := keyMux.PathPrefix("/v2").Subrouter() v1fedmux := fedMux.PathPrefix("/v1").Subrouter() diff --git a/federationapi/routing/send.go b/federationapi/routing/send.go index 436f07e3b9..96932fac2e 100644 --- a/federationapi/routing/send.go +++ b/federationapi/routing/send.go @@ -27,6 +27,7 @@ import ( "github.com/getsentry/sentry-go" "github.com/matrix-org/dendrite/clientapi/jsonerror" eduserverAPI "github.com/matrix-org/dendrite/eduserver/api" + federationAPI "github.com/matrix-org/dendrite/federationapi/api" "github.com/matrix-org/dendrite/internal" keyapi "github.com/matrix-org/dendrite/keyserver/api" "github.com/matrix-org/dendrite/roomserver/api" @@ -101,7 +102,7 @@ func Send( keys gomatrixserverlib.JSONVerifier, federation *gomatrixserverlib.FederationClient, mu *internal.MutexByRoom, - servers ServersInRoomProvider, + servers federationAPI.ServersInRoomProvider, ) util.JSONResponse { t := txnReq{ rsAPI: rsAPI, @@ -160,10 +161,6 @@ func Send( } } -type ServersInRoomProvider interface { - GetServersForRoom(ctx context.Context, roomID string, event *gomatrixserverlib.Event) []gomatrixserverlib.ServerName -} - type txnReq struct { gomatrixserverlib.Transaction rsAPI api.RoomserverInternalAPI @@ -173,7 +170,7 @@ type txnReq struct { federation txnFederationClient roomsMu *internal.MutexByRoom // something that can tell us about which servers are in a room right now - servers ServersInRoomProvider + servers federationAPI.ServersInRoomProvider // a list of events from the auth and prev events which we already had hadEvents map[string]bool // local cache of events for auth checks, etc - this may include events