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

Supports configuration of all TCP Keepalive parameters per https://en.wikipedia.org/wiki/Keepalive #2991

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

hansonchar
Copy link

@hansonchar hansonchar commented Sep 22, 2022

According to https://en.wikipedia.org/wiki/Keepalive, TCP Keepalive has three parameters:

  • Keepalive time is the duration between two keepalive transmissions in idle condition.
  • Keepalive interval is the duration between two successive keepalive retransmissions, if acknowledgement to the previous keepalive transmission is not received.
  • Keepalive retry is the number of retransmissions to be carried out before declaring that remote end is not available.

Currently, however, only the first parameter Keepalive time is configurable in hyper.

This CR is a proposed enhancement for a backward compatible change on the 0.14.x branch so that all the three TCP Keepalive parameters would become configurable.

FYI per empirical evidence enabling TCP keepalive with intervals and retries drastically/significantly improves the stability of persistent TCP connections while reducing the number of file descriptors via closing down dead connections.

@hansonchar
Copy link
Author

I believe I've fixed all the stylistic issues reported by the workflow, but it doesn't seem I can trigger a re-run of the workflow. I also observed some workflow failures unrelated to this change, so they are probably existing failures.

Please let me know if I missed anything.

If possible, I'd like to have this change pushed/released asap, so I can make further PR to axum-server based on this change.

@hansonchar
Copy link
Author

I see there are platform specific conditional compilations in the socket2 crate causing some failure. Looking into them.

@seanmonstar
Copy link
Member

Could just slap #[cfg(unix)] on the extra methods, and not worry about Windows...

@hansonchar
Copy link
Author

hansonchar commented Sep 22, 2022

Could just slap #[cfg(unix)] on the extra methods, and not worry about Windows...

Right, but I am trying to make it such that the same API's can be provided to the upper layer, so that if a API/method exists for a specific parameter that isn't supported on a specific platform, then calling the API/method would just be a no-op. I've just pushed such changes, and added a few unit tests on it. Hopefully they would pass the platform specific tests now.

I used https://docs.rs/socket2/latest/socket2/struct.TcpKeepalive.html as the reference for the platform specific attributes for interval and retries.

@seanmonstar seanmonstar merged commit 287d712 into hyperium:0.14.x Sep 23, 2022
@hansonchar
Copy link
Author

Thanks for merging. Would this change also be merged into the master branch, or is it something that needs a separate PR? Please let me know if I can help.

@seanmonstar
Copy link
Member

The master branch has removed both Server and AddrIncoming. Helpers (such as Tokio integration) will now live in the hyper-util crate.

hansonchar added a commit to hansonchar/hyper-util that referenced this pull request Oct 8, 2022
…tor::tcp_keepalive_retries` (#2991)

    If the platform supports setting the options, otherwise it's a noop.

Port from hyperium/hyper#2991
hansonchar added a commit to hansonchar/hyper-util that referenced this pull request Oct 8, 2022
…tor::tcp_keepalive_retries` (#2991)

    If the platform supports setting the options, otherwise it's a noop.

Port from hyperium/hyper#2991
hansonchar added a commit to hansonchar/hyper-util that referenced this pull request Oct 8, 2022
…tor::tcp_keepalive_retries` (#2991)

    If the platform supports setting the options, otherwise it's a noop.

Port from hyperium/hyper#2991

  hyperium#9
bors bot referenced this pull request in OpenPoolProject/stratum Dec 2, 2022
329: fix(deps): update rust crate hyper to 0.14.23 r=renovate[bot] a=renovate[bot]

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [hyper](https://hyper.rs) ([source](https://github.com/hyperium/hyper)) | dependencies | patch | `0.14.20` -> `0.14.23` |

---

### Release Notes

<details>
<summary>hyperium/hyper</summary>

### [`v0.14.23`](https://github.com/hyperium/hyper/releases/tag/v0.14.23)

[Compare Source](https://github.com/hyperium/hyper/compare/v0.14.22...v0.14.23)

#### Bug Fixes

-   **http2:** Fix race condition in client dispatcher ([#&#8203;3041](https://github.com/hyperium/hyper/issues/3041)) ([2f1c0b72](https://github.com/hyperium/hyper/commit/2f1c0b720da4553fff216a38018a78ecafe23d60), closes [#&#8203;2419](https://github.com/hyperium/hyper/issues/2419))
-   **dependencies**: Really fix compile-time feature for `socket2` dependency.

#### New Contributors

-   [`@&#8203;jfourie1](https://github.com/jfourie1)` made their first contribution in [https://github.com/hyperium/hyper/pull/3041](https://github.com/hyperium/hyper/pull/3041)

### [`v0.14.22`](https://github.com/hyperium/hyper/releases/tag/v0.14.22)

[Compare Source](https://github.com/hyperium/hyper/compare/v0.14.21...v0.14.22)

#### Bug Fixes

-   **server:** fix compile-time cfgs for TCP keepalive options ([#&#8203;3039](https://github.com/hyperium/hyper/issues/3039)) ([e8765e0f](https://github.com/hyperium/hyper/commit/e8765e0febd0267472799dcd1109af75944c2637), closes [#&#8203;3038](https://github.com/hyperium/hyper/issues/3038))

### [`v0.14.21`](https://github.com/hyperium/hyper/releases/tag/v0.14.21)

[Compare Source](https://github.com/hyperium/hyper/compare/v0.14.20...v0.14.21)

#### Bug Fixes

-   **client:** send an error back to client when dispatch misbehaves () ([9fa36382](https://github.com/hyperium/hyper/commit/9fa363829ced232acb18c31ebab8ffb93f691ecc), closes [#&#8203;2649](https://github.com/hyperium/hyper/issues/2649))
-   **http1:** fix `http1_header_read_timeout` to use same future ([#&#8203;2891](https://github.com/hyperium/hyper/issues/2891)) ([c5a14e7c](https://github.com/hyperium/hyper/commit/c5a14e7c087424001223aaeb2dad532ba4ee6063))

#### Features

-   **http1:** allow ignoring invalid header lines in requests ([73dd4746](https://github.com/hyperium/hyper/commit/73dd474652f5e71fe8a87baa6f9b2490ae746eb3))
-   **server:** add `Server::tcp_keepalive_interval` and `Server::tcp_keepalive_retries` ([#&#8203;2991](https://github.com/hyperium/hyper/issues/2991)) ([287d7124](https://github.com/hyperium/hyper/commit/287d712483aec6671427438d60ed2a72f856fd9f))

#### New Contributors

-   [`@&#8203;hansonchar](https://github.com/hansonchar)` made their first contribution in [https://github.com/hyperium/hyper/pull/2991](https://github.com/hyperium/hyper/pull/2991)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/OpenPoolProject/stratum).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC40NC4wIiwidXBkYXRlZEluVmVyIjoiMzQuNDQuMCJ9-->


Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants