Skip to content

fix: ignore ping command in connection keepalive logic #480

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

erobot
Copy link
Contributor

@erobot erobot commented Apr 9, 2025

Fixes #479

Motivation

The current keepalive logic is too lenient – merely receiving ping commands can keep the connection alive.

Modifications

Ignore ping command in connection keepalive logic.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework without any test coverage. It is not easy to write a test for ClientConnection but the change is simple.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    Bug fix only.

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@BewareMyPower BewareMyPower added the bug Something isn't working label Apr 21, 2025
@BewareMyPower BewareMyPower added this to the 3.8.0 milestone Apr 21, 2025
@BewareMyPower BewareMyPower merged commit f37bf92 into apache:main Apr 23, 2025
12 of 13 checks passed
BewareMyPower pushed a commit that referenced this pull request Apr 28, 2025
Fixes #479

### Motivation

The current keepalive logic is too lenient – merely receiving ping commands can keep the connection alive.

### Modifications

Ignore ping command in connection keepalive logic.

(cherry picked from commit f37bf92)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Connection keepalive is not working in certain case
2 participants