Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readonly mode sets node failing on get not existing key #2653

Closed
eskomorokhov opened this issue Jul 19, 2023 · 2 comments
Closed

Readonly mode sets node failing on get not existing key #2653

eskomorokhov opened this issue Jul 19, 2023 · 2 comments

Comments

@eskomorokhov
Copy link

With using READ_ONLY mode and not available master node after first call cluster client returns network err instead of handling commands with available replica.
2 consecutive calls to clusterClient.get("not_existing_key") not existing key from redis-cluster with down master node and available replica differ in result.

Prerequisites redis cluster configured to tolerate not available master and handle requests with replica in READ_ONLY mode.

Expected Behavior

Cluster returns redis:nil for not existing key when READ_ONLY enabled and continue handling requests.
redis-cli -c with READ_ONLY command continue to respond commands with master not available.

Current Behavior

clusterclient return on first get request redis:nil error and on following requests return network error.

Possible Solution

ClusterClient marks node as failing on if ReadOnly is available and cmd returns and error including key not exists(redis:nil).
Possible solution to mark node as failing only on network errors.

Steps to Reproduce

	opt := &ClusterOptions{
		/*
					cluster 9999 is slave of master 6666, other masters 7777,8888
			1d12c247d3e7d8e6bec6e760de568aa89d4d6b19 127.0.0.1:6666@16666 master,nofailover - 1689746329650 1689746326588 7 disconnected 0-5460
			33188d1f5da25a4cce04b1cdf2839bf9b2c2d7d2 127.0.0.1:7777@17777 master - 0 1689746332706 2 connected 5461-10922
			c2d726c0d5312c393a705b14f584af5fbd5506ba 127.0.0.1:9999@19999 myself,slave,nofailover 1d12c247d3e7d8e6bec6e760de568aa89d4d6b19 0 1689746331000 7 connected
			ed983cfbd2023bccb80d195d725f8c64d3eb22c3 127.0.0.1:8888@18888 master - 0 1689746331690 3 connected 10923-16383
		*/
		Addrs:    []string{"localhost:9999"},
		ReadOnly: true,
	}
	client := NewClusterClient(opt)
	cmds, _ := client.Pipelined(ctx, func(pipe Pipeliner) error {
		// k3 key redirects to slot where master set down(6666) and only replica node(9999) available
		_ = pipe.Get(ctx, "k3")
		return nil
	})

	for i, c := range cmds {
		fmt.Printf("#%v cmd %v err: %v\n", i, c.String(), c.Err())
	}

	cmds, _ = client.Pipelined(ctx, func(pipe Pipeliner) error {
		_ = pipe.Get(ctx, "k3")
		return nil
	})

	for i, c := range cmds {
		fmt.Printf("#%v cmd %v err: %v\n", i, c.String(), c.Err())
	}

Context (Environment)

Current behavior prevent using clusterclient and handle get requests with failed master node hence redis-cluster stop handling request if any master node not available.

Detailed Description

Mark node as failing only on network errors

Possible Implementation

@@ -1328,8 +1346,8 @@ func (c *ClusterClient) pipelineReadCmds(
        for i, cmd := range cmds {
                err := cmd.readReply(rd)
                cmd.SetErr(err)
-
-               if err == nil {
+               if err == nil || err == Nil {
                        continue
                }
 
@eskomorokhov eskomorokhov changed the title Readonly mode set node failing on get not existing key Readonly mode sets node failing on get not existing key Jul 19, 2023
@singular-seal
Copy link
Contributor

HI. This is a serious issue which would cause the 'READONLY' mode doesn't work. Why there is no reply on this?

@eskomorokhov
Copy link
Author

Fixed in #2960

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants