Implement Request and Response Policy Based Routing in Cluster Mode #3422

ofekshenawa · 2025-06-30T12:13:18Z

This PR introduces support for Redis COMMAND-based request_policy and response_policy routing for Redis commands when used in OSS Cluster client.

Key Additions:

Command Policy Loader: Parses and caches COMMAND metadata with routing/aggregation tips on first use.
Routing Engine Enhancements:
Implements support for all request policies: default(keyless), default(hashslot), all_shards, all_nodes, multi_shard, and special.
Response Aggregator: Combines multi-shard replies based on response_policy:
all_succeeded, one_succeeded, agg_sum, special, etc.
Includes custom handling for special commands like FT.CURSOR.
Raw Command Support: Policies are enforced on Client.Do(ctx, args...).

* Add search module builders and tests * Add tests

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

htemelski-redis · 2025-09-25T07:23:19Z

osscluster_router.go

+		}
+		if result.cmd != nil && result.err == nil {
+			// For MGET, extract individual values from the array result
+			if strings.ToLower(cmd.Name()) == "mget" {


Do we actually need this special case?

cc @ofekshenawa

osscluster_router.go

.github/workflows/build.yml

ndyakov

Submitting partial review for the aggregators.

internal/routing/aggregator.go

ndyakov · 2025-10-09T11:30:38Z

osscluster_router.go

+	// For MGET without policy, use keyed aggregator
+	if cmdName == "mget" {
+		return routing.NewDefaultAggregator(true)
+	}


Since we are passing the cmd.Name() in routing.NewResponseAggregator this can be handler by it. If policy is nil for mget, maybe the NewResponseAggregator can accept a policy and check the nil as well`.

cc @ofekshenawa , @htemelski-redis

internal/routing/aggregator.go

ndyakov

Submitting another partial review.

internal/routing/policy.go

internal/routing/shard_picker.go

command.go

json.go

osscluster.go

ndyakov

Final part of initial review

Overview:

Let's use atomics when possible.
Left questions related to the node selection and setting of values.

Overall the design of the solution looks good, would have to do an additional pass over the test files once this review is addressed.

Thank you both @ofekshenawa and @htemelski-redis!

osscluster_router.go

ndyakov · 2025-10-09T13:03:40Z

osscluster_router.go

+	if c.hasKeys(cmd) {
+		// execute on key based shard
+		return node.Client.Process(ctx, cmd)
+	}


Do we know that this node servers the slot for the key?

Yes, the node should've been selected based on the slot osscluster.go:L1906

func (c *ClusterClient) cmdNode(

ndyakov · 2025-10-09T13:04:38Z

osscluster_router.go

+		// execute on key based shard
+		return node.Client.Process(ctx, cmd)
+	}
+	return c.executeOnArbitraryShard(ctx, cmd)


since it doesn't matter and there is already some node selected, why not use it?

We have two different ways of picking an arbitrary shard, either round robin or a random one

Yes, I understand that, but for some reason there is already a node selected here that may have been selected because MOVED OR normal key based selection. Why do we have to reselect the node? Shouldn't this selection of arbitrary node be done outside, so we do the node selection only one time and the node on line #52 is the one that should be used for this command?

cc @ofekshenawa , @htemelski-redis

The re-selection is required so keyless commands respect the configured ShardPicker strategy.
cmdNode() picks nodes by hash slot, which is correct for keyed commands but ignores the picker for keyless ones. Calling executeOnArbitraryShard() ensures keyless commands use the user’s picker

But if the command is keyless, what will be the node here? Won't it be picked based on the ShardPolicy? Will it be just a random node? Can't we pick the correct node there?

osscluster_router.go

ndyakov

left some comments related to aggregators

internal/routing/aggregator.go

osscluster.go

ndyakov

The aggregators look good, there are some prints left in the code as bunch of unanswered questions. Let's resolve them before merging this. cc @ofekshenawa , @htemelski-redis

ndyakov · 2025-10-28T15:42:09Z

internal/routing/aggregator.go

+// AggMaxAggregator returns the maximum numeric value from all shards.
+type AggMaxAggregator struct {
+	err atomic.Value
+	res *util.AtomicMax
+}


general question, are those min,max aggregators only for ints?

That's a good question, the initial implementation was for ints only, but looking at the docs, we should support any numerical types

internal/routing/policy.go

internal/routing/shard_picker.go

command.go

ndyakov · 2025-10-28T16:05:18Z

osscluster_router.go

+	// For MGET without policy, use keyed aggregator
+	if cmdName == "mget" {
+		return routing.NewDefaultAggregator(true)
+	}


cc @ofekshenawa , @htemelski-redis

osscluster_router.go

ndyakov · 2025-10-28T16:07:10Z

osscluster_router.go

+
+	defer func() {
+		if r := recover(); r != nil {
+			cmd.SetErr(fmt.Errorf("redis: failed to set command value: %v", r))


why don't we return the error as well? it will return nil, but the err will be set on the cmd.

Not sure, how good of a practice is to modify the return value from within recover

it is possible exactly for scenarios like this one.

.github/workflows/build.yml

* feat: load the policy table in cluster client * Remove comments

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

… load-balance-search-commands-to-shards

…ub.com/ofekshenawa/go-redis into load-balance-search-commands-to-shards

Copilot

Pull Request Overview

This PR adds support for explicit routing policies in Redis Cluster operations by introducing a command policy resolver system. The changes enable commands to be routed to specific shards based on their requirements (all shards, multiple shards, single shard) and aggregate responses accordingly.

Implements a comprehensive routing and aggregation framework for cluster commands
Adds Clone() methods to various command types to support concurrent execution
Introduces configurable shard picker strategies (round-robin, random, static)
Adds extensive test coverage for routing policies and command aggregation

Reviewed Changes

Copilot reviewed 30 out of 46 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
osscluster_router.go	New file implementing command routing logic and response aggregation
osscluster.go	Integration of command policy resolver and routing infrastructure
command_policy_resolver.go	New resolver for determining command routing policies
internal/routing/*.go	Core routing policy types and aggregation implementations
internal/util/atomic_*.go	Atomic min/max utilities for response aggregation
*_commands.go	Added cmdType fields and Clone() methods to command structs
osscluster_test.go	Extensive test coverage for routing policies and aggregation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-09T11:52:09Z

osscluster.go

 }

-func (c *ClusterClient) cmdSlot(cmd Cmder, preferredRandomSlot int) int {
+func (c *ClusterClient) cmdSlot(cmd Cmder, prefferedSlot int) int {


Corrected spelling of 'preffered' to 'preferred'.

Copilot · 2025-11-09T11:52:10Z

osscluster.go

 	}

-	return cmdSlot(cmd, cmdFirstKeyPos(cmd), preferredRandomSlot)
+	return cmdSlot(cmd, cmdFirstKeyPos(cmd), prefferedSlot)


Corrected spelling of 'preffered' to 'preferred'.

ofekshenawa changed the title ~~Load balance search commands to shards~~ Implement Request and Response Policy Based Routing in Cluster Mode Jun 30, 2025

ofekshenawa requested review from bobymicroby, htemelski-redis and ndyakov July 6, 2025 10:28

ofekshenawa marked this pull request as ready for review July 6, 2025 12:54

ofekshenawa and others added 8 commits July 16, 2025 23:06

Add search module builders and tests (#1)

be6dfd5

* Add search module builders and tests * Add tests

Use builders and Actions in more clean way

3dc3051

Merge branch 'master' into master

4e6ec29

Update search_builders.go

e4c4833

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

Update search_builders.go

1928265

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

Apply suggestions from code review

fcf645e

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

Merge branch 'master' into master

f982f11

Merge branch 'master' into master

159c91a

htemelski-redis requested changes Sep 25, 2025

View reviewed changes

htemelski-redis force-pushed the load-balance-search-commands-to-shards branch from 6e3b627 to 1b2eaa6 Compare October 8, 2025 08:05

ndyakov reviewed Oct 9, 2025

View reviewed changes

.github/workflows/build.yml Outdated Show resolved Hide resolved

ndyakov reviewed Oct 9, 2025

View reviewed changes

htemelski-redis force-pushed the load-balance-search-commands-to-shards branch from 727a799 to 14bd6e1 Compare October 14, 2025 07:42

ndyakov reviewed Oct 14, 2025

View reviewed changes

htemelski-redis reviewed Oct 17, 2025

View reviewed changes

osscluster.go Outdated Show resolved Hide resolved

htemelski-redis self-requested a review October 28, 2025 09:32

ndyakov requested changes Oct 28, 2025

View reviewed changes

htemelski-redis force-pushed the load-balance-search-commands-to-shards branch from a4ac8df to 7181bcc Compare October 30, 2025 08:44

htemelski-redis requested a review from dmaier-redislabs as a code owner October 30, 2025 08:44

This comment was marked as resolved.

Sign in to view

ofekshenawa and others added 3 commits October 30, 2025 11:21

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

b2d2d91

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

b943692

* feat: load the policy table in cluster client * Remove comments

modify Tips and command pplicy in commandInfo (#5)

5375c51

htemelski-redis added 6 commits October 30, 2025 11:21

added nil safety assertions

5a510cf

few small refactors

14dde5c

added read only policies

b72becc

removed leftover prints

731505b

Rebased to master, resolved comnflicts

888f791

fixed lint

cd74db0

htemelski-redis force-pushed the load-balance-search-commands-to-shards branch from 07963c2 to cd74db0 Compare October 30, 2025 09:27

htemelski-redis changed the base branch from load-balance-search-commands-to-shards to master October 30, 2025 09:28

htemelski-redis and others added 18 commits October 30, 2025 13:19

updated gha

68f7af8

fixed tests, minor consistency refactor

5c447f9

preallocated simple errors

86c73a0

Merge branch 'master' into load-balance-search-commands-to-shards

79b3cf6

changed numeric aggregators to use float64

71262ec

speculative test fix

79fd0cf

Merge branch 'master' into load-balance-search-commands-to-shards

2084e6c

Update command.go

5131406

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

Update main_test.go

340dcbe

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

Add static shard picker

dbe7509

Merge branch 'master' of https://github.com/ofekshenawa/go-redis into…

68d8dc6

… load-balance-search-commands-to-shards

Merge branch 'master' into load-balance-search-commands-to-shards

ff875b5

Merge branch 'master' into load-balance-search-commands-to-shards

d9a6a19

Merge branch 'load-balance-search-commands-to-shards' of https://gith…

28ec347

…ub.com/ofekshenawa/go-redis into load-balance-search-commands-to-shards

Fix nil value handling in command aggregation

5544624

Modify the Clone method to return a shallow copy

c235f6c

Merge branch 'master' into load-balance-search-commands-to-shards

cc3beaf

Add clone method to digest command

ceeb2c8

ndyakov requested review from Copilot and ndyakov November 9, 2025 11:49

Copilot AI reviewed Nov 9, 2025

View reviewed changes

Optimize keyless command routing to respect ShardPicker policy

93226d0

Implement Request and Response Policy Based Routing in Cluster Mode #3422

Are you sure you want to change the base?

Implement Request and Response Policy Based Routing in Cluster Mode #3422

Uh oh!

Conversation

ofekshenawa commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Additions:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ndyakov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndyakov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndyakov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndyakov Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndyakov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ofekshenawa commented Jun 30, 2025 •

edited

Loading

ndyakov Oct 10, 2025 •

edited

Loading