Skip to content

Commit ec40ae1

Browse files
committed
WIP: Demonstration of potential memberlist optimizations.
This is to show posssible optimizations on the memberlist receive path. I believe both these are correct but I need to spend a little time more double checking. All unit tests pass with both these optimizations. 1) Remove Clone()-ing `ring.Desc` The state is cloned so that when the state is not changed, it is not put back into the kv structure. This is not necessary, as by definition, the state did not change. Additionally, there are no error paths where we depend in this cloning to be able to roll-back the state. 2) Remove copying when normalizing of the `Ingesters` map The costly part here is not normalizing the incoming update, but normalizing the currently held state, as this can be large. By doing this in-place we can avoid a lot of overhead. Both these changes will significantly reduce GC load as both are in essence cloning the ingesters map, only for it to be thrown away shortly after. As a follow up, there might be possibility to avoid the normalization entirely, assuming we can keep the current ring state normalized, but this needs some more reasoning put into it.
1 parent bd9af71 commit ec40ae1

File tree

2 files changed

+38
-2
lines changed

2 files changed

+38
-2
lines changed

kv/memberlist/memberlist_client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1184,7 +1184,7 @@ func (m *KV) mergeValueForKey(key string, incomingValue Mergeable, casVersion ui
11841184
m.storeMu.Lock()
11851185
defer m.storeMu.Unlock()
11861186

1187-
curr := m.store[key].Clone()
1187+
curr := m.store[key]
11881188
// if casVersion is 0, then there was no previous value, so we will just do normal merge, without localCAS flag set.
11891189
if casVersion > 0 && curr.version != casVersion {
11901190
return nil, 0, errVersionMismatch

ring/model.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ func (d *Desc) mergeWithTime(mergeable memberlist.Mergeable, localCAS bool, now
192192
return nil, nil
193193
}
194194

195-
thisIngesterMap := buildNormalizedIngestersMap(d)
195+
normalizeIngestersMap(d)
196+
thisIngesterMap := d.Ingesters
196197
otherIngesterMap := buildNormalizedIngestersMap(other)
197198

198199
var updated []string
@@ -303,6 +304,41 @@ func buildNormalizedIngestersMap(inputRing *Desc) map[string]InstanceDesc {
303304
return out
304305
}
305306

307+
// normalizeIngestersMap will do the following:
308+
// - sorts tokens and removes duplicates (only within single ingester)
309+
// - modifies the input ring
310+
func normalizeIngestersMap(inputRing *Desc) {
311+
for n, ing := range inputRing.Ingesters {
312+
// Make sure LEFT ingesters have no tokens
313+
if ing.State == LEFT {
314+
ing.Tokens = nil
315+
}
316+
317+
// Sort tokens, and remove duplicates
318+
if len(ing.Tokens) == 0 {
319+
inputRing.Ingesters[n] = ing
320+
continue
321+
}
322+
323+
if !sort.IsSorted(Tokens(ing.Tokens)) {
324+
sort.Sort(Tokens(ing.Tokens))
325+
}
326+
327+
// tokens are sorted now, we can easily remove duplicates.
328+
prev := ing.Tokens[0]
329+
for ix := 1; ix < len(ing.Tokens); {
330+
if ing.Tokens[ix] == prev {
331+
ing.Tokens = append(ing.Tokens[:ix], ing.Tokens[ix+1:]...)
332+
} else {
333+
prev = ing.Tokens[ix]
334+
ix++
335+
}
336+
}
337+
338+
inputRing.Ingesters[n] = ing
339+
}
340+
}
341+
306342
func conflictingTokensExist(normalizedIngesters map[string]InstanceDesc) bool {
307343
count := 0
308344
for _, ing := range normalizedIngesters {

0 commit comments

Comments
 (0)