-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
clientv3: fix lease "freezing" on unhealthy cluster #7023
Conversation
mkReqs(1) | ||
clus.Members[1].Stop(t) | ||
mkReqs(2) | ||
time.Sleep(time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q. Why do we wait here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise the leader resumes too fast; couldn't trigger the failure without it
what was the original issue that this pr is trying to fix? |
|
lgtm probably worth to add the explanation #7023 (comment) to the commit message. |
Lack of GRPC code was causing this to look like a halting error to the client.
8eb54e1
to
9be9089
Compare
@@ -574,3 +576,54 @@ func TestLeaseKeepAliveLoopExit(t *testing.T) { | |||
t.Fatalf("expected %T, got %v(%T)", clientv3.ErrKeepAliveHalted{}, err, err) | |||
} | |||
} | |||
|
|||
func TestV3LeaseFailureOverlap(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be nice to add some explanation to why we need this test in the file. so the future reader might have a context.
Other than that LGTM.
Was triggering cancelation errors on outstanding KeepAlives if Grant had to retry.
9be9089
to
a375e91
Compare
No description provided.