Skip to content

Cluster client logs info for command not found #1069

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

Closed
kollektiv opened this issue Jun 26, 2019 · 8 comments · Fixed by #1072
Closed

Cluster client logs info for command not found #1069

kollektiv opened this issue Jun 26, 2019 · 8 comments · Fixed by #1072

Comments

@kollektiv
Copy link

kollektiv commented Jun 26, 2019

Version: 4a3dbb3

Using the cluster client, the internal logger is logging a lot of lines on commands. I believe cmdsInfo is being ran before cluster state has synced, which results in cmdsInfoCache storing an empty result.

When this happens, a random node is always chosen on the first attempt, which has a high chance of getting a MOVED response

Output

redis: 2019/06/26 10:14:07 cluster.go:1534: info for cmd=get not found
redis: 2019/06/26 10:14:07 cluster.go:1534: info for cmd=get not found
redis: 2019/06/26 10:14:07 cluster.go:1534: info for cmd=get not found
redis: 2019/06/26 10:14:07 cluster.go:1534: info for cmd=get not found

Example

package main

import (
	"fmt"

	"github.com/go-redis/redis"
)

func main() {
	client := redis.NewClusterClient(&redis.ClusterOptions{
		Addrs: []string{"localhost:30001"},
	})

	for i := 0; i < 1000; i++ {
		cmd := client.Get("0")
		_, err := cmd.Result()
		if err != nil && err != redis.Nil {
			fmt.Printf("error getting result: %v\n", err)
		}
	}
}
@huija
Copy link

huija commented Jun 27, 2019

github.com/go-redis/redis/tree/v6.15.2
我换成上面的版本没问题了, 应该是有了改动, 文档还是旧的

@vmihailenco
Copy link
Collaborator

I guess that is possible if you say so. But why does it happen?

@lkramer
Copy link

lkramer commented Jun 27, 2019

Hey, I'm seeing this as well. Any idea what the reason is?

@kollektiv
Copy link
Author

@vmihailenco This has started happening since #1056. Before #1056, process would always retrieve the cluster state before through cmdSlotAndNode that called c.state.Get().

Now process is first calling cmdInfo, which calls c.cmdsInfoCache.Get() before cluster state is retrieved

@vmihailenco
Copy link
Collaborator

Okay, but we do we need a cluster state to retrieve commands info? ClusterClient is created with at least 1 healthy node which should be enough to retrieve commands info...

@kollektiv
Copy link
Author

@vmihailenco Cluster state isn't necessary. Refreshing cluster state lazily creates the nodes (c.nodes.GetOrCreate).

cmdsInfo calls c.nodes.Get, so it relies on the nodes being created. What if cmdsInfo calls c.nodes.GetOrCreate instead?

@kollektiv
Copy link
Author

Is it possible to compute the slot of the command without depending on COMMAND?

@vmihailenco
Copy link
Collaborator

What if cmdsInfo calls c.nodes.GetOrCreate instead?

Good call - I've sent a fix.

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.

4 participants