feat(cluster): Implement Request and Response Policy Based Routing in Cluster Mode (#3422)

* Add search module builders and tests (#1)

* Add search module builders and tests

* Add tests

* Use builders and Actions in more clean way

* Update search_builders.go

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* Update search_builders.go

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* feat(routing): add internal request/response policy enums

* feat: load the policy table in cluster client (#4)

* feat: load the policy table in cluster client

* Remove comments

* modify Tips and command pplicy in commandInfo (#5)

* centralize cluster command routing in osscluster_router.go and refactor osscluster.go (#6)

* centralize cluster command routing in osscluster_router.go and refactor osscluster.go

* enalbe ci on all branches

* Add debug prints

* Add debug prints

* FIX: deal with nil policy

* FIX: fixing clusterClient process

* chore(osscluster): simplify switch case

* wip(command): ai generated clone method for commands

* feat: implement response aggregator for Redis cluster commands

* feat: implement response aggregator for Redis cluster commands

* fix: solve concurrency errors

* fix: solve concurrency errors

* return MaxRedirects settings

* remove locks from getCommandPolicy

* Handle MOVED errors more robustly, remove cluster reloading at exectutions, ennsure better routing

* Fix: supports Process hook test

* Fix: remove response aggregation for single shard commands

* Add more preformant type conversion for Cmd type

* Add router logic into processPipeline

---------

Co-authored-by: Nedyalko Dyakov <nedyalko.dyakov@gmail.com>

* remove thread debugging code

* remove thread debugging code && reject commands with policy that cannot be used in pipeline

* refactor processPipline and cmdType enum

* remove FDescribe from cluster tests

* Add tests

* fix aggregation test

* fix mget test

* fix mget test

* remove aggregateKeyedResponses

* added scaffolding for the req-resp manager

* added default policies for the search commands

* split command map into module->command

* cleanup, added logic to refresh the cache

* added reactive cache refresh

* revert cluster refresh

* fixed lint

* addresed first batch of comments

* rewrote aggregator implementations with atomic for native or nearnative primitives

* addressed more comments, fixed lint

* added batch aggregator operations

* fixed lint

* updated batch aggregator, fixed extractcommandvalue

* fixed lint

* added batching to aggregateResponses

* fixed deadlocks

* changed aggregator logic, added error params

* added preemptive return to the aggregators

* more work on the aggregators

* updated and and or aggregators

* fixed lint

* added configurable policy resolvers

* slight refactor

* removed the interface, slight refactor

* change func signature from cmdName to cmder

* added nil safety assertions

* few small refactors

* added read only policies

* removed leftover prints

* Rebased to master, resolved comnflicts

* fixed lint

* updated gha

* fixed tests, minor consistency refactor

* preallocated simple errors

* changed numeric aggregators to use float64

* speculative test fix

* Update command.go

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* Update main_test.go

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>

* Add static shard picker

* Fix nil value handling in command aggregation

* Modify the Clone method to return a shallow copy

* Add clone method to digest command

* Optimize keyless command routing to respect ShardPicker policy

* Remove MGET references

* Fix MGET aggregation to map individual values to keys across shards

* Add clone method to hybrid search commands

* Undo changes in route keyless test

* remove comments

* Add test for DisableRoutingPolicies option

* Add Routing Policies Comprehensive Test Suite and Fix multi keyed aggregation for different step

---------

Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com>
Co-authored-by: Nedyalko Dyakov <nedyalko.dyakov@gmail.com>
Co-authored-by: Hristo Temelski <hristo.temelski@redis.com>

internal/routing/policy.go

@@ -0,0 +1,144 @@
+package routing
+
+import (
+	"fmt"
+	"strings"
+)
+
+type RequestPolicy uint8
+
+const (
+	ReqDefault RequestPolicy = iota
+
+	ReqAllNodes
+
+	ReqAllShards
+
+	ReqMultiShard
+
+	ReqSpecial
+)
+
+const (
+	ReadOnlyCMD string = "readonly"
+)
+
+func (p RequestPolicy) String() string {
+	switch p {
+	case ReqDefault:
+		return "default"
+	case ReqAllNodes:
+		return "all_nodes"
+	case ReqAllShards:
+		return "all_shards"
+	case ReqMultiShard:
+		return "multi_shard"
+	case ReqSpecial:
+		return "special"
+	default:
+		return fmt.Sprintf("unknown_request_policy(%d)", p)
+	}
+}
+
+func ParseRequestPolicy(raw string) (RequestPolicy, error) {
+	switch strings.ToLower(raw) {
+	case "", "default", "none":
+		return ReqDefault, nil
+	case "all_nodes":
+		return ReqAllNodes, nil
+	case "all_shards":
+		return ReqAllShards, nil
+	case "multi_shard":
+		return ReqMultiShard, nil
+	case "special":
+		return ReqSpecial, nil
+	default:
+		return ReqDefault, fmt.Errorf("routing: unknown request_policy %q", raw)
+	}
+}
+
+type ResponsePolicy uint8
+
+const (
+	RespDefaultKeyless ResponsePolicy = iota
+	RespDefaultHashSlot
+	RespAllSucceeded
+	RespOneSucceeded
+	RespAggSum
+	RespAggMin
+	RespAggMax
+	RespAggLogicalAnd
+	RespAggLogicalOr
+	RespSpecial
+)
+
+func (p ResponsePolicy) String() string {
+	switch p {
+	case RespDefaultKeyless:
+		return "default(keyless)"
+	case RespDefaultHashSlot:
+		return "default(hashslot)"
+	case RespAllSucceeded:
+		return "all_succeeded"
+	case RespOneSucceeded:
+		return "one_succeeded"
+	case RespAggSum:
+		return "agg_sum"
+	case RespAggMin:
+		return "agg_min"
+	case RespAggMax:
+		return "agg_max"
+	case RespAggLogicalAnd:
+		return "agg_logical_and"
+	case RespAggLogicalOr:
+		return "agg_logical_or"
+	case RespSpecial:
+		return "special"
+	default:
+		return "all_succeeded"
+	}
+}
+
+func ParseResponsePolicy(raw string) (ResponsePolicy, error) {
+	switch strings.ToLower(raw) {
+	case "default(keyless)":
+		return RespDefaultKeyless, nil
+	case "default(hashslot)":
+		return RespDefaultHashSlot, nil
+	case "all_succeeded":
+		return RespAllSucceeded, nil
+	case "one_succeeded":
+		return RespOneSucceeded, nil
+	case "agg_sum":
+		return RespAggSum, nil
+	case "agg_min":
+		return RespAggMin, nil
+	case "agg_max":
+		return RespAggMax, nil
+	case "agg_logical_and":
+		return RespAggLogicalAnd, nil
+	case "agg_logical_or":
+		return RespAggLogicalOr, nil
+	case "special":
+		return RespSpecial, nil
+	default:
+		return RespDefaultKeyless, fmt.Errorf("routing: unknown response_policy %q", raw)
+	}
+}
+
+type CommandPolicy struct {
+	Request  RequestPolicy
+	Response ResponsePolicy
+	// Tips that are not request_policy or response_policy
+	// e.g nondeterministic_output, nondeterministic_output_order.
+	Tips map[string]string
+}
+
+func (p *CommandPolicy) CanBeUsedInPipeline() bool {
+	return p.Request != ReqAllNodes && p.Request != ReqAllShards && p.Request != ReqMultiShard
+}
+
+func (p *CommandPolicy) IsReadOnly() bool {
+	_, readOnly := p.Tips[ReadOnlyCMD]
+	return readOnly
+}

internal/routing/shard_picker.go

@@ -0,0 +1,57 @@
+package routing
+
+import (
+	"math/rand"
+	"sync/atomic"
+)
+
+// ShardPicker chooses “one arbitrary shard” when the request_policy is
+// ReqDefault and the command has no keys.
+type ShardPicker interface {
+	Next(total int) int // returns an index in [0,total)
+}
+
+// StaticShardPicker always returns the same shard index.
+type StaticShardPicker struct {
+	index int
+}
+
+func NewStaticShardPicker(index int) *StaticShardPicker {
+	return &StaticShardPicker{index: index}
+}
+
+func (p *StaticShardPicker) Next(total int) int {
+	if total == 0 || p.index >= total {
+		return 0
+	}
+	return p.index
+}
+
+/*───────────────────────────────
+   Round-robin (default)
+────────────────────────────────*/
+
+type RoundRobinPicker struct {
+	cnt atomic.Uint32
+}
+
+func (p *RoundRobinPicker) Next(total int) int {
+	if total == 0 {
+		return 0
+	}
+	i := p.cnt.Add(1)
+	return int(i-1) % total
+}
+
+/*───────────────────────────────
+   Random
+────────────────────────────────*/
+
+type RandomPicker struct{}
+
+func (RandomPicker) Next(total int) int {
+	if total == 0 {
+		return 0
+	}
+	return rand.Intn(total)
+}

internal/util/atomic_max.go

@@ -0,0 +1,97 @@
+/*
+© 2023–present Harald Rudell <harald.rudell@gmail.com> (https://haraldrudell.github.io/haraldrudell/)
+ISC License
+
+Modified by htemelski-redis
+Removed the treshold, adapted it to work with float64
+*/
+
+package util
+
+import (
+	"math"
+
+	"go.uber.org/atomic"
+)
+
+// AtomicMax is a thread-safe max container
+//   - hasValue indicator true if a value was equal to or greater than threshold
+//   - optional threshold for minimum accepted max value
+//   - if threshold is not used, initialization-free
+//   - —
+//   - wait-free CompareAndSwap mechanic
+type AtomicMax struct {
+
+	// value is current max
+	value atomic.Float64
+	// whether [AtomicMax.Value] has been invoked
+	// with value equal or greater to threshold
+	hasValue atomic.Bool
+}
+
+// NewAtomicMax returns a thread-safe max container
+//   - if threshold is not used, AtomicMax is initialization-free
+func NewAtomicMax() (atomicMax *AtomicMax) {
+	m := AtomicMax{}
+	m.value.Store((-math.MaxFloat64))
+	return &m
+}
+
+// Value updates the container with a possible max value
+//   - isNewMax is true if:
+//   - — value is equal to or greater than any threshold and
+//   - — invocation recorded the first 0 or
+//   - — a new max
+//   - upon return, Max and Max1 are guaranteed to reflect the invocation
+//   - the return order of concurrent Value invocations is not guaranteed
+//   - Thread-safe
+func (m *AtomicMax) Value(value float64) (isNewMax bool) {
+	//  -math.MaxFloat64 as max case
+	var hasValue0 = m.hasValue.Load()
+	if value == (-math.MaxFloat64) {
+		if !hasValue0 {
+			isNewMax = m.hasValue.CompareAndSwap(false, true)
+		}
+		return // -math.MaxFloat64 as max: isNewMax true for first 0 writer
+	}
+
+	// check against present value
+	var current = m.value.Load()
+	if isNewMax = value > current; !isNewMax {
+		return // not a new max return: isNewMax false
+	}
+
+	// store the new max
+	for {
+
+		// try to write value to *max
+		if isNewMax = m.value.CompareAndSwap(current, value); isNewMax {
+			if !hasValue0 {
+				// may be rarely written multiple times
+				// still faster than CompareAndSwap
+				m.hasValue.Store(true)
+			}
+			return // new max written return: isNewMax true
+		}
+		if current = m.value.Load(); current >= value {
+			return // no longer a need to write return: isNewMax false
+		}
+	}
+}
+
+// Max returns current max and value-present flag
+//   - hasValue true indicates that value reflects a Value invocation
+//   - hasValue false: value is zero-value
+//   - Thread-safe
+func (m *AtomicMax) Max() (value float64, hasValue bool) {
+	if hasValue = m.hasValue.Load(); !hasValue {
+		return
+	}
+	value = m.value.Load()
+	return
+}
+
+// Max1 returns current maximum whether zero-value or set by Value
+//   - threshold is ignored
+//   - Thread-safe
+func (m *AtomicMax) Max1() (value float64) { return m.value.Load() }

internal/util/atomic_min.go

@@ -0,0 +1,96 @@
+package util
+
+/*
+© 2023–present Harald Rudell <harald.rudell@gmail.com> (https://haraldrudell.github.io/haraldrudell/)
+ISC License
+
+Modified by htemelski-redis
+Adapted from the modified atomic_max, but with inverted logic
+*/
+
+import (
+	"math"
+
+	"go.uber.org/atomic"
+)
+
+// AtomicMin is a thread-safe Min container
+//   - hasValue indicator true if a value was equal to or greater than threshold
+//   - optional threshold for minimum accepted Min value
+//   - —
+//   - wait-free CompareAndSwap mechanic
+type AtomicMin struct {
+
+	// value is current Min
+	value atomic.Float64
+	// whether [AtomicMin.Value] has been invoked
+	// with value equal or greater to threshold
+	hasValue atomic.Bool
+}
+
+// NewAtomicMin returns a thread-safe Min container
+//   - if threshold is not used, AtomicMin is initialization-free
+func NewAtomicMin() (atomicMin *AtomicMin) {
+	m := AtomicMin{}
+	m.value.Store(math.MaxFloat64)
+	return &m
+}
+
+// Value updates the container with a possible Min value
+//   - isNewMin is true if:
+//   - — value is equal to or greater than any threshold and
+//   - — invocation recorded the first 0 or
+//   - — a new Min
+//   - upon return, Min and Min1 are guaranteed to reflect the invocation
+//   - the return order of concurrent Value invocations is not guaranteed
+//   - Thread-safe
+func (m *AtomicMin) Value(value float64) (isNewMin bool) {
+	//  math.MaxFloat64 as Min case
+	var hasValue0 = m.hasValue.Load()
+	if value == math.MaxFloat64 {
+		if !hasValue0 {
+			isNewMin = m.hasValue.CompareAndSwap(false, true)
+		}
+		return // math.MaxFloat64 as Min: isNewMin true for first 0 writer
+	}
+
+	// check against present value
+	var current = m.value.Load()
+	if isNewMin = value < current; !isNewMin {
+		return // not a new Min return: isNewMin false
+	}
+
+	// store the new Min
+	for {
+
+		// try to write value to *Min
+		if isNewMin = m.value.CompareAndSwap(current, value); isNewMin {
+			if !hasValue0 {
+				// may be rarely written multiple times
+				// still faster than CompareAndSwap
+				m.hasValue.Store(true)
+			}
+			return // new Min written return: isNewMin true
+		}
+		if current = m.value.Load(); current <= value {
+			return // no longer a need to write return: isNewMin false
+		}
+	}
+}
+
+// Min returns current min and value-present flag
+//   - hasValue true indicates that value reflects a Value invocation
+//   - hasValue false: value is zero-value
+//   - Thread-safe
+func (m *AtomicMin) Min() (value float64, hasValue bool) {
+	if hasValue = m.hasValue.Load(); !hasValue {
+		return
+	}
+	value = m.value.Load()
+	return
+}
+
+// Min1 returns current Minimum whether zero-value or set by Value
+//   - threshold is ignored
+//   - Thread-safe
+func (m *AtomicMin) Min1() (value float64) { return m.value.Load() }