diff --git a/.github/ISSUE_TEMPLATE/BUG_REPORT.md b/.github/ISSUE_TEMPLATE/BUG_REPORT.md index a3b8c0754..e18968baf 100644 --- a/.github/ISSUE_TEMPLATE/BUG_REPORT.md +++ b/.github/ISSUE_TEMPLATE/BUG_REPORT.md @@ -62,6 +62,6 @@ If you can identify any relevant log snippets from server logs, please include those (please be careful to remove any personal or private data). Please surround them with ``` (three backticks, on a line on their own), so that they are formatted legibly. -Alternatively, please send logs to @kegan:matrix.org or @neilalexander:matrix.org +Alternatively, please send logs to @kegan:matrix.org, @s7evink:matrix.org or @devonh:one.ems.host with a link to the respective Github issue, thanks! --> diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index 772b45cb2..ac40f06b0 100644 --- a/.github/workflows/dendrite.yml +++ b/.github/workflows/dendrite.yml @@ -440,7 +440,7 @@ jobs: # Run Complement - run: | set -o pipefail && - go test -v -json -tags dendrite_blacklist ./tests/... 2>&1 | gotestfmt -hide all + go test -v -json -tags dendrite_blacklist ./tests ./tests/csapi 2>&1 | gotestfmt -hide all shell: bash name: Run Complement Tests env: diff --git a/.golangci.yml b/.golangci.yml index bb8d38a8b..5bee0a885 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -180,7 +180,6 @@ linters-settings: linters: enable: - errcheck - - goconst - gocyclo - goimports # Does everything gofmt does - gosimple @@ -211,6 +210,7 @@ linters: - stylecheck - typecheck # Should turn back on soon - unconvert # Should turn back on soon + - goconst # Slightly annoying, as it reports "issues" in SQL statements disable-all: false presets: fast: false diff --git a/build/scripts/complement.sh b/build/scripts/complement.sh index 29feff304..8608d8fa5 100755 --- a/build/scripts/complement.sh +++ b/build/scripts/complement.sh @@ -15,5 +15,5 @@ tar -xzf master.tar.gz # Run the tests! cd complement-master -COMPLEMENT_BASE_IMAGE=complement-dendrite:latest go test -v -count=1 ./tests +COMPLEMENT_BASE_IMAGE=complement-dendrite:latest go test -v -count=1 ./tests ./tests/csapi diff --git a/go.mod b/go.mod index c2ab105b1..839e23f4c 100644 --- a/go.mod +++ b/go.mod @@ -42,12 +42,12 @@ require ( github.com/uber/jaeger-lib v2.4.1+incompatible github.com/yggdrasil-network/yggdrasil-go v0.4.6 go.uber.org/atomic v1.10.0 - golang.org/x/crypto v0.13.0 + golang.org/x/crypto v0.14.0 golang.org/x/exp v0.0.0-20230809150735-7b3493d9a819 golang.org/x/image v0.5.0 golang.org/x/mobile v0.0.0-20221020085226-b36e6246172e golang.org/x/sync v0.3.0 - golang.org/x/term v0.12.0 + golang.org/x/term v0.13.0 gopkg.in/h2non/bimg.v1 v1.1.9 gopkg.in/yaml.v2 v2.4.0 gotest.tools/v3 v3.4.0 @@ -123,8 +123,8 @@ require ( github.com/tidwall/pretty v1.2.1 // indirect go.etcd.io/bbolt v1.3.6 // indirect golang.org/x/mod v0.12.0 // indirect - golang.org/x/net v0.14.0 // indirect - golang.org/x/sys v0.12.0 // indirect + golang.org/x/net v0.17.0 // indirect + golang.org/x/sys v0.13.0 // indirect golang.org/x/text v0.13.0 // indirect golang.org/x/time v0.3.0 // indirect golang.org/x/tools v0.12.0 // indirect diff --git a/go.sum b/go.sum index 7f0e98838..e036f7b2b 100644 --- a/go.sum +++ b/go.sum @@ -354,8 +354,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.13.0 h1:mvySKfSWJ+UKUii46M40LOvyWfN0s2U+46/jDd0e6Ck= -golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= +golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc= +golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -386,8 +386,8 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14= -golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI= +golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= +golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -418,12 +418,12 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o= -golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.12.0 h1:/ZfYdc3zq+q02Rv9vGqTeSItdzZTSNDmfTi0mBAuidU= -golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= +golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek= +golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= diff --git a/userapi/internal/device_list_update.go b/userapi/internal/device_list_update.go index 3fccf56bb..2f33589fe 100644 --- a/userapi/internal/device_list_update.go +++ b/userapi/internal/device_list_update.go @@ -180,11 +180,13 @@ func (u *DeviceListUpdater) Start() error { if err != nil { return err } + + newStaleLists := dedupeStaleLists(staleLists) offset, step := time.Second*10, time.Second - if max := len(staleLists); max > 120 { + if max := len(newStaleLists); max > 120 { step = (time.Second * 120) / time.Duration(max) } - for _, userID := range staleLists { + for _, userID := range newStaleLists { userID := userID // otherwise we are only sending the last entry time.AfterFunc(offset, func() { u.notifyWorkers(userID) @@ -416,6 +418,12 @@ func (u *DeviceListUpdater) worker(ch chan spec.ServerName) { func (u *DeviceListUpdater) processServer(serverName spec.ServerName) (time.Duration, bool) { ctx := u.process.Context() + // If the process.Context is canceled, there is no need to go further. + // This avoids spamming the logs when shutting down + if errors.Is(ctx.Err(), context.Canceled) { + return defaultWaitTime, false + } + logger := util.GetLogger(ctx).WithField("server_name", serverName) deviceListUpdateCount.WithLabelValues(string(serverName)).Inc() @@ -428,13 +436,6 @@ func (u *DeviceListUpdater) processServer(serverName spec.ServerName) (time.Dura return waitTime, true } - defer func() { - for _, userID := range userIDs { - // always clear the channel to unblock Update calls regardless of success/failure - u.clearChannel(userID) - } - }() - for _, userID := range userIDs { userWait, err := u.processServerUser(ctx, serverName, userID) if err != nil { @@ -461,6 +462,11 @@ func (u *DeviceListUpdater) processServer(serverName spec.ServerName) (time.Dura func (u *DeviceListUpdater) processServerUser(ctx context.Context, serverName spec.ServerName, userID string) (time.Duration, error) { ctx, cancel := context.WithTimeout(ctx, requestTimeout) defer cancel() + + // If we are processing more than one user per server, this unblocks further calls to Update + // immediately instead of just after **all** users have been processed. + defer u.clearChannel(userID) + logger := util.GetLogger(ctx).WithFields(logrus.Fields{ "server_name": serverName, "user_id": userID, @@ -579,3 +585,24 @@ func (u *DeviceListUpdater) updateDeviceList(res *fclient.RespUserDevices) error } return nil } + +// dedupeStaleLists de-duplicates the stateList entries using the domain. +// This is used on startup, processServer is getting all users anyway, so +// there is no need to send every user to the workers. +func dedupeStaleLists(staleLists []string) []string { + seenDomains := make(map[spec.ServerName]struct{}) + newStaleLists := make([]string, 0, len(staleLists)) + for _, userID := range staleLists { + _, domain, err := gomatrixserverlib.SplitID('@', userID) + if err != nil { + // non-fatal and should not block starting up + continue + } + if _, ok := seenDomains[domain]; ok { + continue + } + newStaleLists = append(newStaleLists, userID) + seenDomains[domain] = struct{}{} + } + return newStaleLists +} diff --git a/userapi/internal/device_list_update_test.go b/userapi/internal/device_list_update_test.go index 10b9c6521..38fd8b583 100644 --- a/userapi/internal/device_list_update_test.go +++ b/userapi/internal/device_list_update_test.go @@ -428,3 +428,49 @@ func TestDeviceListUpdater_CleanUp(t *testing.T) { } }) } + +func Test_dedupeStateList(t *testing.T) { + alice := "@alice:localhost" + bob := "@bob:localhost" + charlie := "@charlie:notlocalhost" + invalidUserID := "iaminvalid:localhost" + + tests := []struct { + name string + staleLists []string + want []string + }{ + { + name: "empty stateLists", + staleLists: []string{}, + want: []string{}, + }, + { + name: "single entry", + staleLists: []string{alice}, + want: []string{alice}, + }, + { + name: "multiple entries without dupe servers", + staleLists: []string{alice, charlie}, + want: []string{alice, charlie}, + }, + { + name: "multiple entries with dupe servers", + staleLists: []string{alice, bob, charlie}, + want: []string{alice, charlie}, + }, + { + name: "list with invalid userID", + staleLists: []string{alice, bob, invalidUserID}, + want: []string{alice}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := dedupeStaleLists(tt.staleLists); !reflect.DeepEqual(got, tt.want) { + t.Errorf("dedupeStaleLists() = %v, want %v", got, tt.want) + } + }) + } +}