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

eliminates unnecessary ping when the connection is idle #3380

Closed
HuSharp opened this issue Nov 1, 2023 · 0 comments · Fixed by #3381
Closed

eliminates unnecessary ping when the connection is idle #3380

HuSharp opened this issue Nov 1, 2023 · 0 comments · Fixed by #3381
Labels
C-bug Category: bug. Something is wrong. This is bad!

Comments

@HuSharp
Copy link
Contributor

HuSharp commented Nov 1, 2023

Version
master

Platform
Linux 5.4.0-154-generic #171-Ubuntu SMP Fri Jun 16 16:29:04 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Description
Setting PermitWithoutStream to false on the client side eliminates the need to send pings when the connection is idle.

otherwise output too many ping

can check code here https://github.com/grpc/grpc-go/blob/70f1a4045da95b93f73b6dbdd7049f3f053c0680/internal/transport/http2_server.go#L891-L894

grpc-client will check https://github.com/grpc/grpc-go/blob/70f1a4045da95b93f73b6dbdd7049f3f053c0680/internal/transport/http2_client.go#L1685-L1693

I tried this code:

maybe need to check idle here

fn maybe_ping(&mut self, cx: &mut task::Context<'_>, is_idle: bool, shared: &mut Shared) {
        match self.state {
            KeepAliveState::Scheduled(at) => {
                ...
                if !self.while_idle && is_idle {
                    trace!("keep-alive while idle disabled");
                    return;
                }
               ...
            }
            KeepAliveState::Init | KeepAliveState::PingSent => (),
        }
    }

I expected to see this happen: [explanation]
do not output too many ping

Instead, this happened: [explanation]
unnecessary ping from client side

@HuSharp HuSharp added the C-bug Category: bug. Something is wrong. This is bad! label Nov 1, 2023
@HuSharp HuSharp changed the title Setting PermitWithoutStream to false on the client side eliminates the need to send pings when the connection is idle. eliminates unnecessary ping when the connection is idle Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug. Something is wrong. This is bad!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant