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

Netty client doesn't close connection after GOAWAY #9813

Closed
mping-exo opened this issue Jan 12, 2023 · 7 comments
Closed

Netty client doesn't close connection after GOAWAY #9813

mping-exo opened this issue Jan 12, 2023 · 7 comments
Labels

Comments

@mping-exo
Copy link

mping-exo commented Jan 12, 2023

What version of gRPC-Java are you using?

1.51.1

What is your environment?

Ubuntu 22.10
OpenJDK Runtime Environment Temurin-17+35 (build 17+35)

What did you expect to see?

Consistent behaviour between Netty and OkHttp clients when the server is shutting down

What did you see instead?

TCP connections are not closed with Netty after GO_AWAY.
They are closed with OkHttp though.

Steps to reproduce the bug

  1. Start a grpc server for a simple rpc request (we experimented with golang, using java for the server works as expected)
  2. Start a grpc client (in this case, netty)
  3. Client issue an rpc request
  4. Server calls GracefulStop, then replies
    4.1 ongoing RPC from point 3. should be finished
    4.2 after that, server should close tcp connections (server emits two GO_AWAY frames)
  5. Client receives rpc reply
  6. Observe that grpc channel enters IDLE state
  7. Observe that tcp connection never closes with Netty, but does close with OkHttp
  8. If the tcp connection isn't closed, the golang server will block on GracefulStop (this might be a grpc golang issue)

Here are the frames observed from the server side.

I see that a bug might be in golang grpc server (my understanding is that GracefulStop should close tcp connections after inflight RPCs are done, see grpc/grpc-go#5930), but the different behaviour between Netty and OkHttp is puzzling.

I can provide a small sample repo if needed, for now I want to understand if this is expected behaviour or is something that can be configured for the Netty client.

@temawi
Copy link
Contributor

temawi commented Jan 17, 2023

It's not surprising that Netty and OkHttp behave differently as they are completely different implementations.

Ideally the Netty client would close the connection in this situation, but it being a client it doesn't need to be quite as careful with its resources as the server side would and depend on the server to close the connection.

To resolve this particular problem with the Go server not shutting down, I think the focus should be on the server side to make sure the connection gets closed. The bug you linked to seems to me the avenue to pursue.

The Netty client behavior does warrant some investigation and we will keep this bug around to track that work, although it will be somewhat lower in priority.

@temawi temawi added the bug label Jan 17, 2023
@temawi temawi added this to the Next milestone Jan 17, 2023
@ejona86 ejona86 changed the title Different behaviour between Netty and OkHttp clients Netty client doesn't close connection after GOAWAY Jan 17, 2023
@ejona86 ejona86 added the netty label Jan 17, 2023
@mping-exo
Copy link
Author

@temawi Thanks for the reply, I did some tests afterwards with Netty on both the cliend and server, and the behaviour is as expected - connection closes after graceful shutdown. I didn't check who initiated the close though.

@SreeramdasLavanya
Copy link

Hi @mping-exo, @kannanjgithub

I was going through this issue and as part of Analysis found like this issue is already fixed as per the comments mentioned in issue tracker and its working as expected.

Request you to confirm if any further action is required from our side on this issue or we are good to close this?

@mping-exo
Copy link
Author

Good for me, thanks for the heads up.

@SreeramdasLavanya
Copy link

Hi @kannanjgithub,

Requesting you to close this issue as I don't have access to do so.

@kannanjgithub
Copy link
Contributor

@temawi are you ok with closing this issue?

@temawi
Copy link
Contributor

temawi commented Aug 8, 2024

Sounds good, closing the issue.

@temawi temawi closed this as completed Aug 8, 2024
@ejona86 ejona86 removed the bug label Aug 26, 2024
@ejona86 ejona86 removed this from the Next milestone Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants