From d3e2eccb23a3a3f9cd52872c9d29a1f2b27f6c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandor=20Sz=C3=BCcs?= Date: Wed, 25 Mar 2026 13:48:14 +0100 Subject: [PATCH 1/3] fix: valkey updater calculated wrong difference for update required detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sandor Szücs --- net/valkey.go | 8 ++--- net/valkey_test.go | 80 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/net/valkey.go b/net/valkey.go index b54d5971a1..5065adc295 100644 --- a/net/valkey.go +++ b/net/valkey.go @@ -349,6 +349,7 @@ func NewValkeyRingClient(opt *ValkeyOptions) (*ValkeyRingClient, error) { } if err != nil { opt.Log.Errorf("Failed start valkey client: %v", err) + return nil, fmt.Errorf("failed start valkey client: %v", err) } else { opt.Addrs = address } @@ -413,10 +414,10 @@ func (vrc *ValkeyRingClient) startUpdater(ctx context.Context) { addrs, err := vrc.options.AddrUpdater() if err != nil { - vrc.log.Errorf("Failed to run valkey updater: %v", err) + vrc.log.Errorf("Failed to update valkey ring: %v", err) continue } - if !init && len(difference(addrs, old)) != 0 { + if !init && (len(difference(addrs, old)) != 0 || len(difference(old, addrs)) != 0) { vrc.SetAddrs(ctx, addrs) vrc.log.Infof("Valkey updater updated old(%d) -> new(%d)", len(old), len(addrs)) @@ -553,9 +554,9 @@ func difference(a, b []string) []string { result = append(result, item) } } - return result } + func intersect(slice1, slice2 []string) []string { set := make(map[string]struct{}) for _, item := range slice1 { @@ -569,6 +570,5 @@ func intersect(slice1, slice2 []string) []string { delete(set, item) } } - return result } diff --git a/net/valkey_test.go b/net/valkey_test.go index 6dbcd7249e..6dcf806f92 100644 --- a/net/valkey_test.go +++ b/net/valkey_test.go @@ -15,6 +15,86 @@ import ( "github.com/zalando/skipper/tracing/tracingtest" ) +func TestValkeyDifferenceUpdater(t *testing.T) { + for _, tt := range []struct { + name string + a []string + b []string + want int + }{ + { + name: "test a > b", + a: []string{"1s"}, + b: []string{}, + want: 1, + }, + { + name: "test a < b", + a: []string{}, + b: []string{"1s"}, + want: 1, + }, + { + name: "test a = b", + a: []string{"1s"}, + b: []string{"1s"}, + want: 0, + }, + { + name: "test a != b", + a: []string{"1s"}, + b: []string{"2s"}, + want: 2, + }} { + t.Run(tt.name, func(t *testing.T) { + if l, m := len(difference(tt.a, tt.b)), len(difference(tt.b, tt.a)); l+m != tt.want { + t.Errorf("Failed to get correct difference: Want %v", tt.want) + } + }) + } + +} + +func TestValkeyDifference(t *testing.T) { + for _, tt := range []struct { + name string + a []string + b []string + want []string + }{ + { + name: "test a > b", + a: []string{"1s"}, + b: []string{}, + want: []string{"1s"}, + }, + { + name: "test a < b", + a: []string{}, + b: []string{"1s"}, + want: []string{}, + }, + { + name: "test a = b", + a: []string{"1s"}, + b: []string{"1s"}, + want: []string{}, + }, + { + name: "test a != b", + a: []string{"1s"}, + b: []string{"2s"}, + want: []string{"1s"}, + }} { + t.Run(tt.name, func(t *testing.T) { + if got := difference(tt.a, tt.b); len(got) != len(tt.want) { + t.Errorf("Failed to get correct difference: Want %v, got %v", tt.want, got) + } + }) + } + +} + func TestValkeyContainer(t *testing.T) { valkeyAddr, done := valkeytest.NewTestValkey(t) defer done() From 698887a0e5f4c4f4dc2862d66c3fea38d8d9f74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandor=20Sz=C3=BCcs?= Date: Wed, 25 Mar 2026 15:54:49 +0100 Subject: [PATCH 2/3] use slices.Equal and sort.Strings instead of set operation difference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sandor Szücs --- net/valkey.go | 29 +++++++++++++++++++++++------ net/valkey_test.go | 40 ---------------------------------------- 2 files changed, 23 insertions(+), 46 deletions(-) diff --git a/net/valkey.go b/net/valkey.go index 5065adc295..5cbec2ad1f 100644 --- a/net/valkey.go +++ b/net/valkey.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "math" + "slices" + "sort" "strings" "sync" "sync/atomic" @@ -398,12 +400,20 @@ func (vrc *ValkeyRingClient) Close() error { func (vrc *ValkeyRingClient) startUpdater(ctx context.Context) { old := vrc.options.Addrs + sort.Strings(old) vrc.log.Infof("Start goroutine to update valkey instances every %s", vrc.options.UpdateInterval) defer vrc.log.Info("Stopped goroutine to update valkey") ticker := time.NewTicker(vrc.options.UpdateInterval) defer ticker.Stop() + init := true + if len(old) != 0 { + vrc.SetAddrs(ctx, old) + vrc.log.Infof("Valkey updater initial set to %d shards", len(old)) + init = false + } + for { select { @@ -417,15 +427,22 @@ func (vrc *ValkeyRingClient) startUpdater(ctx context.Context) { vrc.log.Errorf("Failed to update valkey ring: %v", err) continue } - if !init && (len(difference(addrs, old)) != 0 || len(difference(old, addrs)) != 0) { + sort.Strings(addrs) + + if init { + if len(addrs) != 0 { + init = false + vrc.SetAddrs(ctx, addrs) + vrc.log.Infof("Valkey updater initial set to %d shards", len(addrs)) + old = addrs + } + continue + } + + if !slices.Equal(old, addrs) { vrc.SetAddrs(ctx, addrs) vrc.log.Infof("Valkey updater updated old(%d) -> new(%d)", len(old), len(addrs)) - old = addrs - } else if init && len(addrs) != 0 { - init = false - vrc.SetAddrs(ctx, addrs) - vrc.log.Infof("Valkey updater initial set to %d shards", len(addrs)) old = addrs } } diff --git a/net/valkey_test.go b/net/valkey_test.go index 6dcf806f92..32f778470e 100644 --- a/net/valkey_test.go +++ b/net/valkey_test.go @@ -15,46 +15,6 @@ import ( "github.com/zalando/skipper/tracing/tracingtest" ) -func TestValkeyDifferenceUpdater(t *testing.T) { - for _, tt := range []struct { - name string - a []string - b []string - want int - }{ - { - name: "test a > b", - a: []string{"1s"}, - b: []string{}, - want: 1, - }, - { - name: "test a < b", - a: []string{}, - b: []string{"1s"}, - want: 1, - }, - { - name: "test a = b", - a: []string{"1s"}, - b: []string{"1s"}, - want: 0, - }, - { - name: "test a != b", - a: []string{"1s"}, - b: []string{"2s"}, - want: 2, - }} { - t.Run(tt.name, func(t *testing.T) { - if l, m := len(difference(tt.a, tt.b)), len(difference(tt.b, tt.a)); l+m != tt.want { - t.Errorf("Failed to get correct difference: Want %v", tt.want) - } - }) - } - -} - func TestValkeyDifference(t *testing.T) { for _, tt := range []struct { name string From 9ad9d9fcefb796f8a16eb7bffb5fad771eb2497a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandor=20Sz=C3=BCcs?= Date: Wed, 25 Mar 2026 17:29:39 +0100 Subject: [PATCH 3/3] revert err handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sandor Szücs --- net/valkey.go | 1 - 1 file changed, 1 deletion(-) diff --git a/net/valkey.go b/net/valkey.go index 5cbec2ad1f..8376bca477 100644 --- a/net/valkey.go +++ b/net/valkey.go @@ -351,7 +351,6 @@ func NewValkeyRingClient(opt *ValkeyOptions) (*ValkeyRingClient, error) { } if err != nil { opt.Log.Errorf("Failed start valkey client: %v", err) - return nil, fmt.Errorf("failed start valkey client: %v", err) } else { opt.Addrs = address }