diff --git a/CHANGELOG.md b/CHANGELOG.md index f356d4caa..529acb1b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,5 +18,5 @@ * [ENHANCEMENT] Replace go-kit/kit/log with go-kit/log. #52 * [ENHANCEMENT] Add spanlogger package. #42 * [ENHANCEMENT] Add runutil.CloseWithLogOnErr function. #58 -* [ENHANCEMENT] Optimise memberlist receive path when used as a backing store for rings with a large number of members. #76 +* [ENHANCEMENT] Optimise memberlist receive path when used as a backing store for rings with a large number of members. #76 #77 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 diff --git a/kv/memberlist/mergeable.go b/kv/memberlist/mergeable.go index e636e8c80..2c02acfa4 100644 --- a/kv/memberlist/mergeable.go +++ b/kv/memberlist/mergeable.go @@ -8,6 +8,8 @@ type Mergeable interface { // Merge with other value in place. Returns change, that can be sent to other clients. // If merge doesn't result in any change, returns nil. // Error can be returned if merging with given 'other' value is not possible. + // Implementors of this method are permitted to modify the other parameter, as the + // memberlist-based KV store will not use the same "other" parameter in multiple Merge calls. // // In order for state merging to work correctly, Merge function must have some properties. When talking about the // result of the merge in the following text, we don't mean the return value ("change"), but the diff --git a/ring/model.go b/ring/model.go index cb2d7c787..ca5e85ff4 100644 --- a/ring/model.go +++ b/ring/model.go @@ -173,6 +173,8 @@ func (i *InstanceDesc) IsReady(now time.Time, heartbeatTimeout time.Duration) er // (see resolveConflicts). // // This method is part of memberlist.Mergeable interface, and is only used by gossiping ring. +// +// Note: This method modifies d and mergeable to reduce allocations and copies. func (d *Desc) Merge(mergeable memberlist.Mergeable, localCAS bool) (memberlist.Mergeable, error) { return d.mergeWithTime(mergeable, localCAS, time.Now()) } @@ -192,8 +194,11 @@ func (d *Desc) mergeWithTime(mergeable memberlist.Mergeable, localCAS bool, now return nil, nil } - thisIngesterMap := buildNormalizedIngestersMap(d) - otherIngesterMap := buildNormalizedIngestersMap(other) + normalizeIngestersMap(d) + normalizeIngestersMap(other) + + thisIngesterMap := d.Ingesters + otherIngesterMap := other.Ingesters var updated []string @@ -261,22 +266,18 @@ func (d *Desc) MergeContent() []string { return result } -// buildNormalizedIngestersMap will do the following: +// normalizeIngestersMap will do the following: // - sorts tokens and removes duplicates (only within single ingester) -// - it doesn't modify input ring -func buildNormalizedIngestersMap(inputRing *Desc) map[string]InstanceDesc { - out := map[string]InstanceDesc{} - +// - modifies the input ring +func normalizeIngestersMap(inputRing *Desc) { // Make sure LEFT ingesters have no tokens for n, ing := range inputRing.Ingesters { if ing.State == LEFT { ing.Tokens = nil + inputRing.Ingesters[n] = ing } - out[n] = ing - } - // Sort tokens, and remove duplicates - for name, ing := range out { + // Sort tokens, and remove duplicates if len(ing.Tokens) == 0 { continue } @@ -297,10 +298,8 @@ func buildNormalizedIngestersMap(inputRing *Desc) map[string]InstanceDesc { } // write updated value back to map - out[name] = ing + inputRing.Ingesters[n] = ing } - - return out } func conflictingTokensExist(normalizedIngesters map[string]InstanceDesc) bool {