Skip to content
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

Automated cherry pick of #10468 on release-3.4 #11299

Conversation

jingyih
Copy link
Contributor

@jingyih jingyih commented Oct 28, 2019

Cherry pick of #10468 on release-3.4.

#10468: etcdserver: remove auth validation loop

Remove auth validation loop in v3_server.raftRequest(). Re-validation
when error ErrAuthOldRevision occurs should be handled on client side.
Disable TestV3AuthOldRevConcurrent for now. See
etcd-io#10468 (comment)
@jingyih jingyih changed the title Automated cherry pick of #10468 Automated cherry pick of #10468 on release-3.4 Oct 28, 2019
@jingyih
Copy link
Contributor Author

jingyih commented Oct 28, 2019

@mitake @gyuho
#10218 was included in 3.4 release. Let's also cherry pick this. So that on server side there is no auth loop on error ErrAuthOldRevision.

@gyuho
Copy link
Contributor

gyuho commented Oct 28, 2019

lgtm,

can we highlight this change? (e.g. "please handle err auth old revision error in client side")

@jingyih
Copy link
Contributor Author

jingyih commented Oct 28, 2019

can we highlight this change? (e.g. "please handle err auth old revision error in client side")

From client's perspective, they need to handle ErrAuthOldRevision w/ or w/o this change. The only difference is that w/o the change, server hangs with all CPU cycles used by the loop.

@jingyih
Copy link
Contributor Author

jingyih commented Oct 28, 2019

fmt test failed:

Earlier we merged commit bb5ba14 which has a bad commit title. This will make all future PRs in this branch fail the fmt test.

@xqzhang2015
Copy link

@jingyih is this PR blocked by CI?

@jingyih
Copy link
Contributor Author

jingyih commented Nov 6, 2019

can we highlight this change? (e.g. "please handle err auth old revision error in client side")

From client's perspective, they need to handle ErrAuthOldRevision w/ or w/o this change. The only difference is that w/o the change, server hangs with all CPU cycles used by the loop.

@gyuho Given that it is always the case that "client needs to handle ErrAuthOldRevision", do you think this PR needs a changelog highlight?

@jingyih jingyih merged commit b66203c into etcd-io:release-3.4 Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants