clientv3/integration: add TestBalancerUnderServerShutdownMutable* #8772
Conversation
|
|
||
| timeout := time.Second | ||
| failed := false | ||
| for i := 0; i < 30; i++ { |
There was a problem hiding this comment.
why do we need 30 retries? we should expect exact one retry, right?
There was a problem hiding this comment.
I had 30 retries in case request never fails (try up to ~3-sec), or the error is transport is closing which takes more than 100ms to recover(endpoint switch)--so it takes more than 1-retry to succeed.
Changed the logic to test with only one retry after failure, by giving more wait time.
gyuho
force-pushed
the
shutdown
branch
2 times, most recently
from
253db0e
to
a690357
Compare
| // shut down eps[0] | ||
| clus.Members[0].Terminate(t) | ||
|
|
||
| // switched to others when eps[0] was shut down |
|
lgtm |
|
/cc @jpbetz |
| } | ||
|
|
||
| // testBalancerUnderServerShutdownMutable expects when the member of the | ||
| // pinned endpoint is shut down, balancer switch its endpoints and following |
There was a problem hiding this comment.
grammar nit: "...expects that when the member of the pinned endpoint is shut down, the balancer switches its endpoints and all subsequent..."
|
|
||
| // switched to others when eps[0] was explicitly shut down | ||
| // and following request should succeed | ||
| time.Sleep(time.Second) |
There was a problem hiding this comment.
Potential source of test flakiness? Is there a condition we can poll for that would be more reliable?
Signed-off-by: Gyu-Ho Lee <[email protected]>
|
@jpbetz All fixed. And added |
|
lgtm Thanks! |
#8711