client: call .Endpoints() in dial() in client/v3/client.go instead of accessing cfg.Endpoints directly#13203
Conversation
|
I agree that this variable needs a lock to prevent data race with Instead of of just adding lock, maybe we should consider adding dedicated endpoint field to |
|
yeah, I agree. I only fixed what our race detector complained but didn't read the code in detail. There is one thing that if |
|
nvm, we don't do deep copy in SetEndpoints |
|
any updates, or should I do something? |
… accessing cfg.Endpoints directly https://github.com/etcd-io/etcd/blob/0cdd558361c6bdbbd9e4023558e2f6ece71c18ad/client/v3/client.go#L299 accesses endpoints without acquiring lock. Fix it to call Endpoints() Fix etcd-io#13201
|
squash commits |
Thanks! |
…lient/v3/client.go instead of accessing cfg.Endpoints directly Signed-off-by: Chao Chen <chaochn@amazon.com>
…lient/v3/client.go instead of accessing cfg.Endpoints directly Signed-off-by: Chao Chen <chaochn@amazon.com>
|
Now that this PR is available in etcd 3.6, there's some GRPC warnings being issued when creating a client via client.New: Is that expected? Everything else works as intended, |
| client.cancel() | ||
| return nil, fmt.Errorf("at least one Endpoint is required in client config") | ||
| } | ||
| client.SetEndpoints(cfg.Endpoints...) |
There was a problem hiding this comment.
This calls client.resolver.SetEndpoints(eps), which hasn't been called before. I cannot really judge if this is a problem or not, but I observe GRPC warnings being logged with this:
2025/10/15 12:55:29 WARNING: [core] [Channel #1 SubChannel #2] grpc: addrConn.createTransport failed to connect to {Addr: "127.0.0.1:2379", ServerName: "127.0.0.1:2379", }. Err: connection error: desc = "transport: Error while dialing: dial tcp 127.0.0.1:2379: operation was canceled"
or
2025/10/15 12:56:24 WARNING: [core] [Channel #1 SubChannel #2] grpc: addrConn.createTransport failed to connect to {Addr: "127.0.0.1:2379", ServerName: "127.0.0.1:2379", }. Err: connection error: desc = "transport: authentication handshake failed: context canceled"
Replacing this with the following makes them disappear again:
| client.SetEndpoints(cfg.Endpoints...) | |
| client.epMu.Lock() | |
| client.endpoints = cfg.Endpoints | |
| client.epMu.Unlock() |
etcd/client/v3/client.go
Line 299 in 0cdd558
endpoints without acquiring lock. Fix it to call Endpoints()
Fix #13201