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

InitAddress issue when connecting to a valkey cluster using NewClient() #9

Open
moment1027 opened this issue Jul 24, 2024 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@moment1027
Copy link

When connecting to a valkey cluster using NewClient(), if InitAddress is hostname rather than ip, the hostname and the ip for hostname are queried when Nodes() are called as shown below.

[InitValkey] InitValkey, conf:{Addr:[valkey-master-1:6379] }
[InitValkey] Nodes Address:valkey-master-1:6379, Nodes:&{conn:0x400011e2c0 stop:0 cmd:{ks:16384} retry:true DisableCache:false}

[InitValkey] Nodes Address:172.19.0.10:6379, Nodes:&{conn:0x400052a2c0 stop:0 cmd:{ks:16384} retry:true DisableCache:false} <= valkey-master-1's IP address
[InitValkey] Nodes Address:172.19.0.12:6379, Nodes:&{conn:0x400052a160 stop:0 cmd:{ks:16384} retry:true DisableCache:false}
[InitValkey] Nodes Address:172.19.0.13:6379, Nodes:&{conn:0x400052a000 stop:0 cmd:{ks:16384} retry:true DisableCache:false}

[InitValkey] Nodes Address:172.19.0.9:6379, Nodes:&{conn:0x400052a0b0 stop:0 cmd:{ks:16384} retry:true DisableCache:false}
[InitValkey] Nodes Address:172.19.0.11:6379, Nodes:&{conn:0x4000326000 stop:0 cmd:{ks:16384} retry:true DisableCache:false}
[InitValkey] Nodes Address:172.19.0.14:6379, Nodes:&{conn:0x400052a210 stop:0 cmd:{ks:16384} retry:true DisableCache:false}

Therefore, when you query the Cluster Node using Nodes(), the data is duplicated.
// master
[Keys] address:valkey-master-1:6379, keys:[key_3]
[Keys] address:172.19.0.10:6379, keys:[key_3]
[Keys] address:172.19.0.12:6379, keys:[key_2]
[Keys] address:172.19.0.13:6379, keys:[key_1 key_4 key_0]

// slave
[Keys] address:172.19.0.9:6379, keys:[]
[Keys] address:172.19.0.11:6379, keys:[]
[Keys] address:172.19.0.14:6379, keys:[]

@moment1027
Copy link
Author

moment1027 commented Jul 24, 2024

The cause is the code below.
cluster.go, line:242

// make sure InitAddress always be present
for _, addr := range c.opt.InitAddress {
    if _, ok := conns[addr]; !ok {
        conns[addr] = connrole{
            conn: c.connFn(addr, c.opt),
        }
    }
}

@rueian
Copy link
Collaborator

rueian commented Jul 24, 2024

Hi @moment1027, thanks for reporting! This is indeed caused by the code you mentioned and can be easily fixed by adding a new field to the connrole struct, for example:

// make sure InitAddress always be present
for _, addr := range c.opt.InitAddress {
    if _, ok := conns[addr]; !ok {
        conns[addr] = connrole{
            conn: c.connFn(addr, c.opt),
            hidden: true, // <- add this, and filter by this when using `Nodes()`
        }
    }
}

Would you like to open a PR to https://github.com/redis/rueidis?

@rueian rueian added bug Something isn't working good first issue Good for newcomers labels Jul 26, 2024
@moment1027
Copy link
Author

moment1027 commented Jul 29, 2024

Is there any particular reason for initaddress to be saved in conns[] as hidden?
I don't understand the whole logic, so I don't know the impact that can occur when I simply filter in Nodes().

Is there any way to make separate calls to master and slave nodes like redis-go in cluster mode?

@rueian
Copy link
Collaborator

rueian commented Jul 29, 2024

Is there any particular reason for initaddress to be saved in conns[] as hidden?

Initial addresses are always saved so that the client can always get topology from them. If initial addresses are not saved, they can be forgotten by the client when topology changes, and then the client is out of your control until you restart it with the same initial addresses.

I don't understand the whole logic, so I don't know the impact that can occur when I simply filter in Nodes().

I believe there is no impact.

Is there any way to make separate calls to master and slave nodes like redis-go in cluster mode?

Could you elaborate more on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants