-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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/integration: add blackhole tests for range RPCs #8790
Conversation
02ee4b4
to
bac6a43
Compare
bac6a43
to
86e72a7
Compare
newly added tests are failing. |
8cfee6e
to
ea73348
Compare
func TestBalancerUnderBlackholeNoKeepAliveLinearizableGetShortTimeout(t *testing.T) { | ||
testBalancerUnderBlackholeNoKeepAlive(t, func(cli *clientv3.Client, ctx context.Context) error { | ||
_, err := cli.Get(ctx, "a") | ||
if err == context.DeadlineExceeded { |
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.
is this correct? l-get needs to go through raft, wont etcd return errtimeout when the leader is changing?
func TestBalancerUnderBlackholeNoKeepAliveLinearizableGetLongTimeout(t *testing.T) { | ||
testBalancerUnderBlackholeNoKeepAlive(t, func(cli *clientv3.Client, ctx context.Context) error { | ||
_, err := cli.Get(ctx, "a") | ||
if err == context.DeadlineExceeded { |
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.
same 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.
and what is the difference between the longtimeout and shorttimeout one for l-get?
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.
Yeah, old one was wrong. We expect time-out errors on blackholed node, regardless of context timeout duration. Just removed it. Now only tests if it can switch out of blackholed endpoint after context timeout errors.
ea73348
to
a63307e
Compare
Signed-off-by: Gyu-Ho Lee <[email protected]>
a63307e
to
8d23e1c
Compare
lgtm if test passes. |
Fix #8711