Renew gRPC and REST connections after max_connection_age - #6547
Conversation
34ce5f1 to
204b588
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 204b588544
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
204b588 to
fca4b8e
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/quickwit-oss/quickwit/blob/fca4b8e845340dbd9dfca7bbbafd13cfc4befaba/quickwit-integration-tests/src/tests/tls_tests.rs#L402
This assertion assumes that quickwit_serve::reload_tls_cert() has completed by the time the max-age connection closes, but that call only notifies the background reload task and returns before the blocking reload necessarily runs. On a slow CI worker the reconnect can still negotiate server1_der and fail this test even though the feature works; the hot-reload test below avoids this race by polling until the rotated certificate is served.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # cert_poll_interval: 5m | ||
| # # Maximum lifetime of a connection before the server sends an HTTP/2 GOAWAY and the | ||
| # # client reconnects. Disabled when unset. | ||
| # max_connection_age: 30m |
There was a problem hiding this comment.
These sample entries are indented under tls: (same level as cert_poll_interval), but the new fields are parsed as rest.max_connection_age/grpc.max_connection_age siblings of tls, not as members of TlsConfig. Since TlsConfig denies unknown fields, uncommenting this documented snippet (and the analogous gRPC lines below) makes startup fail with an unknown max_connection_age under tls; move both max-age keys out to the rest/grpc level.
Useful? React with 👍 / 👎.
2 participants
Description
Renew gRPC and REST connections after
max_connection_agewhen set so hot-reloads of mTLS certs, keys, etc. takes effect.How was this PR tested?
Added unit and integration tests