leizhiyuan opened a new pull request, #1479:
URL: https://github.com/apache/rocketmq-clients/pull/1479
### Which Issue(s) This PR Fixes
Fixes #1478
### Brief Description
Opening the telemetry stream had no upper bound.
`establish_telemetry_stream` signs the request with `sign_without_timeout` on
purpose, because a `grpc-timeout` would end the long-lived stream, but as a
side effect the wait for the response headers of `self.stub.telemetry(request)`
was not bounded either:
- a peer that accepts the connection but never speaks was only cut off by
the HTTP/2 keep-alive (`KEEP_ALIVE_INTERVAL` 60s + `KEEP_ALIVE_TIMEOUT` 30s);
- a peer that still acks the pings (e.g. the proxy's telemetry handler is
stuck before its first `onNext`) kept the caller waiting forever.
This affects `Producer::start` and every first RPC to an endpoint, and
`SessionManager::get_or_create_session` holds the session map lock meanwhile,
so other RPCs queue behind it.
This PR bounds only the wait for the response of the telemetry call by
`ClientOption::timeout`, as suggested in the issue; once the server answered,
the stream stays long-lived. `Session::start`, `Session::reconnect` and
`Session::update_settings` all go through `establish_telemetry_stream` and are
bounded by the same change. A timeout is reported as `ClientInternal` at
`session.start` with the message `rpc telemetry timed out` and the `peer` /
`timeout` in the context; other telemetry errors are reported as before.
The session map lock granularity (#1448 / #1454) is out of scope; this
change only bounds how long the lock can be held here.
### How Did You Test This Change?
Added two tests in `rust/src/session.rs`, against a local tonic server that
completes the HTTP/2 handshake and acks pings but never handles the RPC:
- `session_start_fails_within_timeout_when_telemetry_is_never_answered`: the
server never sends the response headers; `start` must fail within
`ClientOption::timeout`. Before the fix it stayed pending until the 10s guard
of the test.
- `telemetry_stream_outlives_timeout_once_answered`: the server sends the
headers and keeps the stream open; the stream must not be closed after
`ClientOption::timeout`. Checked that it fails if the telemetry request carries
a `grpc-timeout`.
The reproduction of #1478 (`ClientOption::timeout` = 3s), before / after:
```
silent_tcp: returned after 90.009680217s: Err("... send rpc telemetry
failed, source: status: Unavailable, message: \"http2 error: keep-alive timed
out\" ...")
silent_h2: still pending after 120.001172491s
```
```
silent_h2: returned after 3.002195299s: Err("Client internal error at
client.start, context: { peer: 127.0.0.1:34185, timeout: 3s, called:
session.start } => rpc telemetry timed out")
silent_tcp: returned after 3.009530452s: Err("Client internal error at
client.start, context: { peer: 127.0.0.1:39007, timeout: 3s, called:
session.start } => rpc telemetry timed out")
```
Locally:
- `cargo test`: all passed (lib: 183 passed)
- `cargo fmt --check`, `cargo clippy --all-features -- -D warnings`, `cargo
doc --no-deps --all-features`: passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
