server: drop read request if the leader is changed #10094
Conversation
etcdserver/server.go
Outdated
| @@ -938,7 +941,9 @@ func (s *EtcdServer) run() { | |||
| s.compactor.Resume() | |||
| } | |||
| } | |||
|
|
|||
| if newLeader { | |||
| s.leaderChanged <- struct{}{} | |||
There was a problem hiding this comment.
Handle when linearizableReadLoop had returned from a stopped server?
select {
case s.leaderChanged <- struct{}{}:
default:
}
etcdserver/server.go
Outdated
|
|
||
| if newLeader { | ||
| select { | ||
| case s.leaderChanged <- struct{}{}: |
There was a problem hiding this comment.
if we want to do a broadcast style notification, we should close this channel not just sending one item.
There was a problem hiding this comment.
What do you mean? seems I do not need to broadcast the leader change, it just consumes the channel in linearizableReadLoop. There are two possibilities of being blocked in the loop, one is no read request before the leader changed, and another is that there is a read request coming during the leader changed process. Although there are two places to consume this channel, it only consumes once. cc @xiang90
There was a problem hiding this comment.
The chan is named as leaderChanged. By reading the variable name, people will assume they will get notification when there is a leader change by using this chan. However, when this chan gets used by multiple routines, this chan only deliver one notification. There will be a problem.
I would rather make this chan a broadcast chan.
nolouch
force-pushed
the
drop-read
branch
2 times, most recently
from
4c0b00a to
694ea2c
Compare
|
The general change looks good to me. defer to @gyuho after fixing the CI issue. |
|
Thanks @xiang90. Will take a look at CIs this week. |
|
@gyuho can you check the CI when you have time? thanks! |
|
@xiang90 There were some flaky tests from clientv3. Not related. Merging on greens. Thanks! |
|
@gyuho Thanks! |
The read request will block the following read request until the context timeout if meet network partition, and this PR will drop the read request if the leader is changed. ref: #9567 #9566