From 2583ff05f74d423fd77499d1d23e71e61f95b453 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sat, 30 Nov 2024 22:59:52 +0000 Subject: [PATCH 1/2] Fix ordering issues in requesting backfill Using a map means the orders are processed in an indeterminate order, which causes a lot of `/state_ids` requests as the `prev_events` are not known when the order is lost. Also topologically sort results from multiple servers when doing a `RequestBackfill` just to be safe, since otherwise results retrieved from multiple servers could end up out-of-order too. Signed-off-by: Neil Alexander --- roomserver/internal/perform/perform_backfill.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/roomserver/internal/perform/perform_backfill.go b/roomserver/internal/perform/perform_backfill.go index 8ad255c2..e01b2488 100644 --- a/roomserver/internal/perform/perform_backfill.go +++ b/roomserver/internal/perform/perform_backfill.go @@ -128,9 +128,9 @@ func (r *Backfiller) backfillViaFederation(ctx context.Context, req *api.Perform logrus.WithError(err).WithField("room_id", req.RoomID).Infof("backfilled %d events", len(events)) // persist these new events - auth checks have already been done - roomNID, backfilledEventMap := persistEvents(ctx, r.DB, r.Querier, events) + roomNID, storedEvents := persistEvents(ctx, r.DB, r.Querier, events) - for _, ev := range backfilledEventMap { + for _, ev := range storedEvents { // now add state for these events stateIDs, ok := requester.eventIDToBeforeStateIDs[ev.EventID()] if !ok { @@ -591,10 +591,10 @@ func joinEventsFromHistoryVisibility( return evs, visibility, err } -func persistEvents(ctx context.Context, db storage.Database, querier api.QuerySenderIDAPI, events []gomatrixserverlib.PDU) (types.RoomNID, map[string]types.Event) { +func persistEvents(ctx context.Context, db storage.Database, querier api.QuerySenderIDAPI, events []gomatrixserverlib.PDU) (types.RoomNID, []types.Event) { var roomNID types.RoomNID var eventNID types.EventNID - backfilledEventMap := make(map[string]types.Event) + storedEvents := make([]types.Event, 0, len(events)) for j, ev := range events { nidMap, err := db.EventNIDs(ctx, ev.AuthEventIDs()) if err != nil { // this shouldn't happen as RequestBackfill already found them @@ -647,10 +647,10 @@ func persistEvents(ctx context.Context, db storage.Database, querier api.QuerySe ev = redactedEvent events[j] = ev } - backfilledEventMap[ev.EventID()] = types.Event{ + storedEvents = append(storedEvents, types.Event{ EventNID: eventNID, PDU: ev, - } + }) } - return roomNID, backfilledEventMap + return roomNID, storedEvents } From 56bc4a906aaf45694082c020e81054a083c8a60c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 18 Feb 2025 17:36:29 +0000 Subject: [PATCH 2/2] Fix `golangci-lint` linter configuration Signed-off-by: Neil Alexander --- .golangci.yml | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6f3fd362..2b280e1b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,7 +28,6 @@ run: # the dependency descriptions in go.mod. #modules-download-mode: (release|readonly|vendor) - # output configuration options output: # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" @@ -41,7 +40,6 @@ output: # print linter name in the end of issue text, default is true print-linter-name: true - # all available settings of specific linters linters-settings: errcheck: @@ -72,22 +70,12 @@ linters-settings: - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf - golint: - # minimal confidence for issues, default is 0.8 - min-confidence: 0.8 gofmt: # simplify code: gofmt with `-s` option, true by default simplify: true - goimports: - # put imports beginning with prefix after 3rd-party packages; - # it's a comma-separated list of prefixes - #local-prefixes: github.com/org/project gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) min-complexity: 25 - maligned: - # print struct with more effective memory layout or not, false by default - suggest-new: true dupl: # tokens count to trigger issue, 150 by default threshold: 100 @@ -96,30 +84,17 @@ linters-settings: min-len: 3 # minimal occurrences count to trigger, 3 by default min-occurrences: 3 - depguard: - list-type: blacklist - include-go-root: false - packages: - # - github.com/davecgh/go-spew/spew misspell: # Correct spellings using locale preferences for US or UK. # Default is to use a neutral variety of English. # Setting locale to US will correct the British spelling of 'colour' to 'color'. locale: UK - ignore-words: - # - someword lll: # max line length, lines longer will be reported. Default is 120. # '\t' is counted as 1 character by default, and can be changed with the tab-width option line-length: 96 # tab width in spaces. Default to 1. tab-width: 1 - unused: - # treat code as a program (not a library) and report unused exported identifiers; default is false. - # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: - # if it's called for subdir of a project it can't find funcs usages. All text editor integrations - # with golangci-lint call it on a directory with the changed file. - check-exported: false unparam: # Inspect exported functions, default is false. Set to true if no external program/library imports your code. # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: @@ -167,7 +142,6 @@ linters: - gosimple - govet - ineffassign - - megacheck - misspell # Check code comments, whereas misspell in CI checks *.md files - nakedret - staticcheck @@ -182,22 +156,16 @@ linters: - gochecknoinits - gocritic - gofmt - - golint - gosec # Should turn back on soon - - interfacer - lll - - maligned - prealloc # Should turn back on soon - - scopelint - 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 - issues: # which files to skip: they will be analyzed, but issues from them # won't be reported. Default value is empty list, but there is @@ -217,13 +185,6 @@ issues: - bin - docs - # List of regexps of issue texts to exclude, empty list by default. - # But independently from this option we use default exclude patterns, - # it can be disabled by `exclude-use-default: false`. To list all - # excluded by default patterns execute `golangci-lint run --help` - exclude: - # - abcdef - # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: # Exclude some linters from running on tests files.