From e482fc1f721a55ec3df649704ff859d6641227e4 Mon Sep 17 00:00:00 2001 From: Steve Simpson Date: Tue, 23 Nov 2021 09:11:07 +0100 Subject: [PATCH] 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 the 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. Instead of doing this, we should just ensure that when the state is updated, it is kept in "normalized" form. This might already be the case, but should be verified. 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. --- kv/memberlist/memberlist_client.go | 2 +- ring/model.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kv/memberlist/memberlist_client.go b/kv/memberlist/memberlist_client.go index 1b21fa5c4..61d0813ab 100644 --- a/kv/memberlist/memberlist_client.go +++ b/kv/memberlist/memberlist_client.go @@ -1184,7 +1184,7 @@ func (m *KV) mergeValueForKey(key string, incomingValue Mergeable, casVersion ui m.storeMu.Lock() defer m.storeMu.Unlock() - curr := m.store[key].Clone() + curr := m.store[key] //.Clone() // if casVersion is 0, then there was no previous value, so we will just do normal merge, without localCAS flag set. if casVersion > 0 && curr.version != casVersion { return nil, 0, errVersionMismatch diff --git a/ring/model.go b/ring/model.go index cb2d7c787..2df1a988b 100644 --- a/ring/model.go +++ b/ring/model.go @@ -192,7 +192,7 @@ func (d *Desc) mergeWithTime(mergeable memberlist.Mergeable, localCAS bool, now return nil, nil } - thisIngesterMap := buildNormalizedIngestersMap(d) + thisIngesterMap := d.Ingesters //buildNormalizedIngestersMap(d) otherIngesterMap := buildNormalizedIngestersMap(other) var updated []string