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

server: v1.45 of grpc returns context cancelled on server shutdown #5773

Closed
DarrylWong opened this issue Mar 21, 2022 · 1 comment
Closed
Labels
triage me I really want to be triaged.

Comments

@DarrylWong
Copy link

DarrylWong commented Mar 21, 2022

When trying to upgrade GCS packages, upgrading from v1.43 to. v1.45 of grpc causes the test TestDirectoryConnect/drain_connection to fail.

This test fails now as draining the server no longer does anything. Looking into it, drain() does not do anything as there is no event listener ever added to the server. No event listener is added as the watchPods() in line 292 of directory.go shuts down before the second server, tds2, can start up.

The reason why the handler shuts down earlier than it should be is due to the following lines in watchPods():

if grpcutil.IsContextCanceled(err) {
break
}

where IsContextCanceled() is:

func IsContextCanceled(err error) bool {
if s, ok := status.FromError(errors.UnwrapAll(err)); ok {
return s.Code() == codes.Canceled && s.Message() == context.Canceled.Error()
}
return false
}

In v1.44 of grpc and before, when the context was canceled from grpc's side, it would return an unknown error. As of v1.45, it now returns a context.Canceled error. As a result, IsContextCanceled() now returns true even if the context was canceled by the servers side, which makes the two currently indistinguishable. The way the code currently works is that it only wants to break out if the context was canceled from our end, otherwise, try starting up the watchPods() handler again.

This functionality is now broken as stopping the server in line 707 of TestDirectoryConnect/drain_connection incorrectly stops the watchPods() handler as stopping the server returns a context.Canceled error.

@DarrylWong DarrylWong added the triage me I really want to be triaged. label Mar 21, 2022
@DarrylWong
Copy link
Author

oops made this in wrong place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

1 participant