diff --git a/.github/codecov.yaml b/.github/codecov.yaml index 78122c990..3462e91ee 100644 --- a/.github/codecov.yaml +++ b/.github/codecov.yaml @@ -7,7 +7,7 @@ coverage: project: default: target: auto - threshold: 0% + threshold: 0.1% base: auto flags: - unittests diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index 558418a6f..5235e9092 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -630,6 +630,7 @@ func handleGuestRegistration( AccessToken: token, IPAddr: req.RemoteAddr, UserAgent: req.UserAgent(), + FromRegistration: true, }, &devRes) if err != nil { return util.JSONResponse{ @@ -919,6 +920,7 @@ func completeRegistration( DeviceID: deviceID, IPAddr: ipAddr, UserAgent: userAgent, + FromRegistration: true, }, &devRes) if err != nil { return util.JSONResponse{ diff --git a/federationapi/routing/backfill.go b/federationapi/routing/backfill.go index 75a007265..bc4138839 100644 --- a/federationapi/routing/backfill.go +++ b/federationapi/routing/backfill.go @@ -95,6 +95,12 @@ func Backfill( } } + // Enforce a limit of 100 events, as not to hit the DB to hard. + // Synapse has a hard limit of 100 events as well. + if req.Limit > 100 { + req.Limit = 100 + } + // Query the Roomserver. if err = rsAPI.PerformBackfill(httpReq.Context(), &req, &res); err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("query.PerformBackfill failed") diff --git a/roomserver/api/perform.go b/roomserver/api/perform.go index 2818efaa3..9e00da2c0 100644 --- a/roomserver/api/perform.go +++ b/roomserver/api/perform.go @@ -8,7 +8,6 @@ import ( "github.com/matrix-org/dendrite/roomserver/types" "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" - "github.com/matrix-org/util" ) type PerformCreateRoomRequest struct { @@ -91,14 +90,44 @@ type PerformBackfillRequest struct { VirtualHost spec.ServerName `json:"virtual_host"` } -// PrevEventIDs returns the prev_event IDs of all backwards extremities, de-duplicated in a lexicographically sorted order. +// limitPrevEventIDs is the maximum of eventIDs we +// return when calling PrevEventIDs. +const limitPrevEventIDs = 100 + +// PrevEventIDs returns the prev_event IDs of either 100 backwards extremities or +// len(r.BackwardsExtremities). Limited to 100, due to Synapse/Dendrite stopping after reaching +// this limit. (which sounds sane) func (r *PerformBackfillRequest) PrevEventIDs() []string { - var prevEventIDs []string - for _, pes := range r.BackwardsExtremities { - prevEventIDs = append(prevEventIDs, pes...) + var uniqueIDs map[string]struct{} + + // Create a unique eventID map of either 100 or len(r.BackwardsExtremities). + // 100 since Synapse/Dendrite stops after reaching 100 events. + if len(r.BackwardsExtremities) > limitPrevEventIDs { + uniqueIDs = make(map[string]struct{}, limitPrevEventIDs) + } else { + uniqueIDs = make(map[string]struct{}, len(r.BackwardsExtremities)) } - prevEventIDs = util.UniqueStrings(prevEventIDs) - return prevEventIDs + +outerLoop: + for _, pes := range r.BackwardsExtremities { + for _, evID := range pes { + uniqueIDs[evID] = struct{}{} + // We found enough unique eventIDs. + if len(uniqueIDs) >= limitPrevEventIDs { + break outerLoop + } + } + } + + // map -> []string + result := make([]string, len(uniqueIDs)) + i := 0 + for evID := range uniqueIDs { + result[i] = evID + i++ + } + + return result } // PerformBackfillResponse is a response to PerformBackfill. diff --git a/roomserver/api/perform_test.go b/roomserver/api/perform_test.go new file mode 100644 index 000000000..f26438d32 --- /dev/null +++ b/roomserver/api/perform_test.go @@ -0,0 +1,81 @@ +package api + +import ( + "fmt" + "math/rand" + "testing" + + "github.com/stretchr/testify/assert" +) + +func BenchmarkPrevEventIDs(b *testing.B) { + for _, x := range []int64{1, 10, 100, 500, 1000, 2000} { + benchPrevEventIDs(b, int(x)) + } +} + +func benchPrevEventIDs(b *testing.B, count int) { + bwExtrems := generateBackwardsExtremities(b, count) + backfiller := PerformBackfillRequest{ + BackwardsExtremities: bwExtrems, + } + + b.Run(fmt.Sprintf("Original%d", count), func(b *testing.B) { + b.ResetTimer() + for i := 0; i < b.N; i++ { + prevIDs := backfiller.PrevEventIDs() + _ = prevIDs + } + }) +} + +type testLike interface { + Helper() +} + +const randomIDCharsCount = 10 + +func generateBackwardsExtremities(t testLike, count int) map[string][]string { + t.Helper() + result := make(map[string][]string, count) + for i := 0; i < count; i++ { + eventID := randomEventId(int64(i)) + result[eventID] = []string{ + randomEventId(int64(i + 1)), + randomEventId(int64(i + 2)), + randomEventId(int64(i + 3)), + } + } + return result +} + +const alphanumerics = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" + +// randomEventId generates a pseudo-random string of length n. +func randomEventId(src int64) string { + randSrc := rand.NewSource(src) + b := make([]byte, randomIDCharsCount) + for i := range b { + b[i] = alphanumerics[randSrc.Int63()%int64(len(alphanumerics))] + } + return string(b) +} + +func TestPrevEventIDs(t *testing.T) { + // generate 10 backwards extremities + bwExtrems := generateBackwardsExtremities(t, 10) + backfiller := PerformBackfillRequest{ + BackwardsExtremities: bwExtrems, + } + + prevIDs := backfiller.PrevEventIDs() + // Given how "generateBackwardsExtremities" works, this + // generates 12 unique event IDs + assert.Equal(t, 12, len(prevIDs)) + + // generate 200 backwards extremities + backfiller.BackwardsExtremities = generateBackwardsExtremities(t, 200) + prevIDs = backfiller.PrevEventIDs() + // PrevEventIDs returns at max 100 event IDs + assert.Equal(t, 100, len(prevIDs)) +} diff --git a/userapi/api/api.go b/userapi/api/api.go index a0dce9758..d4daec820 100644 --- a/userapi/api/api.go +++ b/userapi/api/api.go @@ -379,6 +379,10 @@ type PerformDeviceCreationRequest struct { // update for this account. Generally the only reason to do this is if the account // is an appservice account. NoDeviceListUpdate bool + + // FromRegistration determines if this request comes from registering a new account + // and is in most cases false. + FromRegistration bool } // PerformDeviceCreationResponse is the response for PerformDeviceCreation @@ -803,6 +807,10 @@ type PerformUploadKeysRequest struct { // itself doesn't change but it's easier to pretend upload new keys and reuse the same code paths. // Without this flag, requests to modify device display names would delete device keys. OnlyDisplayNameUpdates bool + + // FromRegistration is set if this key upload comes right after creating an account + // and determines if we need to inform downstream components. + FromRegistration bool } // PerformUploadKeysResponse is the response to PerformUploadKeys diff --git a/userapi/internal/key_api.go b/userapi/internal/key_api.go index 786a2dcd8..422898c70 100644 --- a/userapi/internal/key_api.go +++ b/userapi/internal/key_api.go @@ -711,9 +711,15 @@ func (a *UserInternalAPI) uploadLocalDeviceKeys(ctx context.Context, req *api.Pe } return } - err = emitDeviceKeyChanges(a.KeyChangeProducer, existingKeys, keysToStore, req.OnlyDisplayNameUpdates) - if err != nil { - util.GetLogger(ctx).Errorf("Failed to emitDeviceKeyChanges: %s", err) + + // If the request does _not_ come right after registering an account + // inform downstream components. However, we're fine with just creating the + // database entries above in other cases. + if !req.FromRegistration { + err = emitDeviceKeyChanges(a.KeyChangeProducer, existingKeys, keysToStore, req.OnlyDisplayNameUpdates) + if err != nil { + util.GetLogger(ctx).Errorf("Failed to emitDeviceKeyChanges: %s", err) + } } } diff --git a/userapi/internal/user_api.go b/userapi/internal/user_api.go index 4e3c2671a..a126dc871 100644 --- a/userapi/internal/user_api.go +++ b/userapi/internal/user_api.go @@ -316,7 +316,7 @@ func (a *UserInternalAPI) PerformDeviceCreation(ctx context.Context, req *api.Pe return nil } // create empty device keys and upload them to trigger device list changes - return a.deviceListUpdate(dev.UserID, []string{dev.ID}) + return a.deviceListUpdate(dev.UserID, []string{dev.ID}, req.FromRegistration) } func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.PerformDeviceDeletionRequest, res *api.PerformDeviceDeletionResponse) error { @@ -356,10 +356,10 @@ func (a *UserInternalAPI) PerformDeviceDeletion(ctx context.Context, req *api.Pe return fmt.Errorf("a.KeyAPI.PerformDeleteKeys: %w", err) } // create empty device keys and upload them to delete what was once there and trigger device list changes - return a.deviceListUpdate(req.UserID, deletedDeviceIDs) + return a.deviceListUpdate(req.UserID, deletedDeviceIDs, false) } -func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) error { +func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string, fromRegistration bool) error { deviceKeys := make([]api.DeviceKeys, len(deviceIDs)) for i, did := range deviceIDs { deviceKeys[i] = api.DeviceKeys{ @@ -371,8 +371,9 @@ func (a *UserInternalAPI) deviceListUpdate(userID string, deviceIDs []string) er var uploadRes api.PerformUploadKeysResponse if err := a.PerformUploadKeys(context.Background(), &api.PerformUploadKeysRequest{ - UserID: userID, - DeviceKeys: deviceKeys, + UserID: userID, + DeviceKeys: deviceKeys, + FromRegistration: fromRegistration, }, &uploadRes); err != nil { return err }