etcdserver: use context for Renew #6929
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
heyitsanthony
force-pushed
the
ctx-lease-renew
branch
3 times, most recently
from
1928a61 to
2c3ba79
Compare
|
lgtm. Defer to @xiang90 |
xiang90
reviewed
Dec 6, 2016
| <-ka | ||
|
|
||
| // force keepalive stream message to timeout | ||
| clus.Members[1].Stop(t) |
There was a problem hiding this comment.
how do we ensure member[0] is not the leader?
There was a problem hiding this comment.
leader will stepdown after X seconds I believe, not immediately.
There was a problem hiding this comment.
OK, but for the duration it's not going to be able to expire any keys. The test just needs to get a renew failure + lease extension. Maybe this test should be renamed TestLeaseRenewLostQuorum?
xiang90
reviewed
Dec 6, 2016
| if err != nil { | ||
| return -1, err | ||
| } | ||
| req = req.WithContext(ctx) |
There was a problem hiding this comment.
ugh 3.0 backporting, I'll fix this to do it without it
heyitsanthony
force-pushed
the
ctx-lease-renew
branch
from
2c3ba79 to
f7ffbf1
Compare
added 3 commits
December 6, 2016 14:09
Giving Renew() the default request timeout causes TestV3LeaseFailover to miss its timing constraints. Since it only needs to wait until the leader recognizes the leader is lost, use RequireLeader to cancel the keepalive stream before the request times out.
Would retry a few times before returning a not primary error that the client should never see. Instead, use proper timeouts and then return a request timeout error on failure. Fixes etcd-io#6922
heyitsanthony
force-pushed
the
ctx-lease-renew
branch
from
f7ffbf1 to
be1f36d
Compare
|
lgtm |
|
I assume this is to be backported if 1.6 compatibility is important |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Renew would retry a few times before returning a not primary error that
the client should never see. Instead, use proper timeouts and
then return a request timeout error on failure.
Fixes #6922