etcdserver: remove infinite loop for auth in raftRequest

[jingyih]

Remove auth validation loop in v3_server.raftRequest(). Re-validation when error ErrAuthOldRevision occurs should be handled on client side.

This is a follow up to #10218. The client side implementation is tracked in #10408.

[jingyih]

Do we still need to distinguish between raftRequest() and raftRequestOnce()?

[jingyih]

/cc @xiang90

[jingyih]

Just saw there is a failed test TestV3AuthOldRevConcurrent, I will take a look.

[codecov-io]

Codecov Report

Merging #10468 into master will increase coverage by 0.34%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #10468      +/-   ##
==========================================
+ Coverage   64.18%   64.53%   +0.34%     
==========================================
  Files         403      403              
  Lines       37956    37953       -3     
==========================================
+ Hits        24362    24492     +130     
+ Misses      11963    11809     -154     
- Partials     1631     1652      +21

Impacted Files	Coverage Δ
etcdserver/v3_server.go	72.22% <0%> (-1.03%)	⬇️
client/keys.go	70.35% <0%> (-21.11%)	⬇️
clientv3/auth.go	53.24% <0%> (-6.5%)	⬇️
pkg/testutil/recorder.go	77.77% <0%> (-3.71%)	⬇️
clientv3/ordering/kv.go	82.35% <0%> (-2.95%)	⬇️
pkg/adt/interval_tree.go	84.71% <0%> (-2.51%)	⬇️
etcdserver/api/v3rpc/watch.go	77.45% <0%> (-2.29%)	⬇️
clientv3/retry.go	69.64% <0%> (-1.79%)	⬇️
clientv3/retry_interceptor.go	69.5% <0%> (-1%)	⬇️
... and 23 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 669cd6b...41539df. Read the comment docs.

[mitake]

@jingyih thanks for handling this! I took a look about the failed test. The failed reason after removing the loop is that simple token always uses the latest revision (see tokenSimple.info()) and it isn't working as a validation mechanism (simple token is just for testing purposes, but probably we should fix it). So the test relies on and checks the wrong validation and retry mechanism.
I think disabling the test for now is ok. After moving the validation to client side, it should be revived. How do you think?

[jingyih]

Thanks @mitake for the explanation. I am worried that we might forget to re-enable the test if we disable it now. Maybe we can keep this PR open until the client side retry get implemented.

[jingyih]

Disable integration test TestV3AuthOldRevConcurrent, until #10408 is fixed.

[jingyih]

@mitak @gyuho PTAL.
I think we should get this merged. Once ErrAuthOldRevision happens, this loop will utilize all the CPU resource, as reported in #11033 and #10408.

[mitake]

LGTM!