The implementations of Election methods in ElectionImpl.java do not do auth retries: they do not use the execute method in Impl, which calls into the failsafe retry policy, instead they use the completable method, which doesn't.
The result for a follower that may be blocking in a campaign request for more than 5 minutes (the default time it takes for an auth token to expire) is, when promoted to leader, its campaign call will fail right away with "etcdserver: invalid auth token".
The implementation of Election methods (in ElectionImpl.java) does not set the metadata headers to require leader. I believe it should, since participating or observing an election that becomes frozen client side due to a partitioned server defies the purpose of implementing fault tolerance on top of these primitives (also see Additional context section).
To Reproduce
Call Election.campaign and arrange for it to block after some other already elected leader, wait 5 minutes, kill the leader and instead of succeeding the campaign call will fail with "etcdserver: invalid auth token".
Expected behavior
The methods in Election should retry like every other API implementation method (eg, KV.get) does.
When the same situation with a follower that waits more than 5 minutes blocked in campaign and then becomes leader is simulated using etcdctl, the error shows as a warning but the call is automatically retried by the golang implementation:
# The DH_ETCD_DIR env var below sets up some role for authorization in a script we use to wrap etcdctl.
$ DH_ETCD_DIR=/etc/sysconfig/illumon.d/etcd/client/root etcdctl.sh elect /pepo/ 1
{"level":"warn","ts":"2024-03-11T17:33:03.864-0400","logger":"etcd-client","caller":"v3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"etcd-endpoints://0xc000398c40/10.128.0.201:2379","attempt":0,"error":"rpc error: code = Unauthenticated desc = etcdserver: invalid auth token"}
/pepo//14d08e2f6108dd67
1
Additional context
Looking at the go client sources, it seems the go client (and by implication etcdctl) does not use the concurrency gRPC API, and instead directly implements the campaign and related calls as etcd operations (eg, campaign is implemented as a transaction and a watch, here: https://github.com/etcd-io/etcd/blob/e7b3bb6ccac840770f108ef9a0f013fa51b83256/client/v3/concurrency/election.go#L74) instead of making unary concurrency proto RPCs. On one hand this lends evidence to the use of require leader for calling concurrency RPCs (since the get that is part of the transaction for campaign is a linearized read, etc). On another hand, this is worrisome as it means the concurrency gRPC API is probably not as well tested; the behaviors are slightly different, eg, in the direct implementation in go client the blocking is implemented by waiting on responses on a watch, which is a streaming RPC, while in the RPC version of campaign is the unary campaign RPC itself that is blocking. The server side implementation of concurrency RPC calls ends up calling the same go code in the client (eg, the transaction + watch for campaign, etc), so the low level implementation is the same, but the details of the RPC calls involved in clients (eg, go etcd versus jetcd) are different.
Also note that in go, adding "require leader" is an operation done on the gRPC context, and the campaign calls in go get that as a parameter, so it is possible for a caller to set that, and on second thought they should, since even if the transaction itself is going to require leader, the watcher does not by default I think. In our case in jetcd there is no way to add require leader in the API. My thought is that in both cases, go and jetcd, it should not be optional, given the use case for leader election methods, ie, implementing fault tolerance features. I don't think optional makes sense in this context. The same is probably true for lock. so perhaps we should do something there too.
Versions
Describe the bug
The implementations of Election methods in
ElectionImpl.javado not do auth retries: they do not use theexecutemethod inImpl, which calls into the failsafe retry policy, instead they use thecompletablemethod, which doesn't. The result for a follower that may be blocking in a campaign request for more than 5 minutes (the default time it takes for an auth token to expire) is, when promoted to leader, its campaign call will fail right away with "etcdserver: invalid auth token".The implementation of Election methods (in
ElectionImpl.java) does not set the metadata headers to require leader. I believe it should, since participating or observing an election that becomes frozen client side due to a partitioned server defies the purpose of implementing fault tolerance on top of these primitives (also see Additional context section).To Reproduce Call
Election.campaignand arrange for it to block after some other already elected leader, wait 5 minutes, kill the leader and instead of succeeding the campaign call will fail with "etcdserver: invalid auth token".Expected behavior The methods in Election should retry like every other API implementation method (eg, KV.get) does. When the same situation with a follower that waits more than 5 minutes blocked in campaign and then becomes leader is simulated using etcdctl, the error shows as a warning but the call is automatically retried by the golang implementation:
Additional context Looking at the go client sources, it seems the go client (and by implication etcdctl) does not use the concurrency gRPC API, and instead directly implements the campaign and related calls as etcd operations (eg, campaign is implemented as a transaction and a watch, here: https://github.com/etcd-io/etcd/blob/e7b3bb6ccac840770f108ef9a0f013fa51b83256/client/v3/concurrency/election.go#L74) instead of making unary concurrency proto RPCs. On one hand this lends evidence to the use of require leader for calling concurrency RPCs (since the get that is part of the transaction for campaign is a linearized read, etc). On another hand, this is worrisome as it means the concurrency gRPC API is probably not as well tested; the behaviors are slightly different, eg, in the direct implementation in go client the blocking is implemented by waiting on responses on a watch, which is a streaming RPC, while in the RPC version of campaign is the unary campaign RPC itself that is blocking. The server side implementation of concurrency RPC calls ends up calling the same go code in the client (eg, the transaction + watch for campaign, etc), so the low level implementation is the same, but the details of the RPC calls involved in clients (eg, go etcd versus jetcd) are different.
Also note that in go, adding "require leader" is an operation done on the gRPC context, and the campaign calls in go get that as a parameter, so it is possible for a caller to set that, and on second thought they should, since even if the transaction itself is going to require leader, the watcher does not by default I think. In our case in jetcd there is no way to add require leader in the API. My thought is that in both cases, go and jetcd, it should not be optional, given the use case for leader election methods, ie, implementing fault tolerance features. I don't think optional makes sense in this context. The same is probably true for lock. so perhaps we should do something there too.