From 3e3cca726fabb24c56c475a8fb7e21dbfef420ef Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Thu, 4 Jan 2024 22:19:29 +0100 Subject: [PATCH 1/2] Fix multiple lock acquire on membership update --- common/membership/hashring.go | 14 +++++++++++--- common/membership/hashring_test.go | 5 ++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/common/membership/hashring.go b/common/membership/hashring.go index 66eec3fe8e9..7b2abe828a9 100644 --- a/common/membership/hashring.go +++ b/common/membership/hashring.go @@ -28,6 +28,7 @@ import ( "github.com/dgryski/go-farm" "github.com/uber/ringpop-go/hashring" + "github.com/uber/ringpop-go/membership" "github.com/uber/cadence/common" "github.com/uber/cadence/common/log" @@ -239,9 +240,8 @@ func (r *ring) refresh() error { } ring := emptyHashring() - for _, member := range members { - ring.AddMembers(member) - } + ring.AddMembers(castToMembers(members)...) + r.members.keys = newMembersMap r.members.refreshed = time.Now() r.value.Store(ring) @@ -292,3 +292,11 @@ func (r *ring) compareMembers(members []HostInfo) (map[string]HostInfo, bool) { } return newMembersMap, changed } + +func castToMembers[T membership.Member](members []T) []membership.Member { + result := make([]membership.Member, 0, len(members)) + for _, h := range members { + result = append(result, h) + } + return result +} diff --git a/common/membership/hashring_test.go b/common/membership/hashring_test.go index 61f3cdf4883..2564eb6daa2 100644 --- a/common/membership/hashring_test.go +++ b/common/membership/hashring_test.go @@ -31,7 +31,6 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" - "github.com/uber/cadence/common" "github.com/uber/cadence/common/log" ) @@ -218,7 +217,7 @@ func TestLookupAndRefreshRaceCondition(t *testing.T) { wg.Add(2) go func() { for i := 0; i < 50; i++ { - hr.Lookup("a") + _, _ = hr.Lookup("a") } wg.Done() }() @@ -226,7 +225,7 @@ func TestLookupAndRefreshRaceCondition(t *testing.T) { for i := 0; i < 50; i++ { // to bypass internal check hr.members.refreshed = time.Now().AddDate(0, 0, -1) - hr.refresh() + assert.NoError(t, hr.refresh()) } wg.Done() }() From 17758a5beb9eb5ce806d7b7404c78fe3c9ee4a51 Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jan 2024 09:55:57 +0100 Subject: [PATCH 2/2] fmt --- common/membership/hashring_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/common/membership/hashring_test.go b/common/membership/hashring_test.go index 2564eb6daa2..639777131a8 100644 --- a/common/membership/hashring_test.go +++ b/common/membership/hashring_test.go @@ -31,6 +31,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/uber/cadence/common" "github.com/uber/cadence/common/log" )