Ring: reduce allocations and copies when Merge()-ing states. (#77)

Instead of creating copies of the "Ingesters" maps when normalizing, normalize
them in-place. This reduces the overhead of creating new maps only to discard
the source versions shortly after.

Two additional changes yielded also yielded some improvement:
- When checking for LEFT states, don't update the map unless necessary.
- Collapsing the two loops into one, avoiding iterating the map twice.

```
name                             old time/op    new time/op    delta
MemberlistReceiveWithRingDesc-6    1.01ms ± 6%    0.44ms ±15%  -56.56%  (p=0.000 n=9+9)

name                             old alloc/op   new alloc/op   delta
MemberlistReceiveWithRingDesc-6     254kB ± 0%      16kB ± 0%  -93.85%  (p=0.000 n=10+9)

name                             old allocs/op  new allocs/op  delta
MemberlistReceiveWithRingDesc-6       637 ± 0%       612 ± 0%   -3.92%  (p=0.000 n=10+9)
```

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

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

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 {