You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "TomPlanche (via GitHub)" <gi...@apache.org> on 2026/10/12 18:09:00 UTC
[PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
TomPlanche opened a new pull request, #4501:
URL: https://github.com/apache/iggy/pull/4501
## Which issue does this PR address?
Closes #4327
## Rationale
The TCP, QUIC and WebSocket clients of the Rust SDK wedge after 1000 diagnostic events. As @BrandonArp reports on #4327, a client whose server stays down reaches this limit on its own, because each failed reconnect sweep publishes `Disconnected`. @hubcio suggested overflow mode on the issue.
## What changed?
Each client kept the receiver of its bounded event channel so that `subscribe_events()` can clone it, and nothing read that receiver. The channel frees a slot only when every receiver has read it, so after 1000 events the next publish waited forever, inside the connect that sent it. Every later call on the client then waited behind that connect.
Now one helper, `diagnostic_event_channel()`, creates the channel in overflow mode for the TCP, QUIC, WebSocket and HTTP clients. A full channel drops its oldest event instead of waiting, so a subscriber that falls behind loses old events, not the newest ones. The producer and the consumer read the channel as a `Stream`, which skips the overflow error, so their event loops keep running.
## Local Execution
The build and the tests ran on macOS.
- Passed: `cargo fmt --all`, `cargo sort --no-format --workspace` and `cargo clippy --all-features --all-targets -- -D warnings`.
- Passed: `cargo test -p iggy --lib`, with 291 tests.
- Passed: the two new tests fail without overflow mode. The TCP test makes 1001 failed connects to a closed port, as in the reproduction on #4327, and stops at the same point:
```text
connect 1001 hung
```
- Passed: the `sdk::` integration suite (77 passed), because the producer and the consumer read these events.
- Passed: `prek run --files` on the changed files.
## AI Usage
1. Claude Code.
2. It wrote the helper, the change in the four clients and the two tests, from the fix suggested on #4327.
3. Both new tests fail without `set_overflow(true)` and pass with it. I checked in async-broadcast 0.7.2 that the `Stream` impl skips `Overflowed`, so the producer and consumer loops do not stop. The suites above pass.
4. Yes.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #4501:
URL: https://github.com/apache/iggy/pull/4501
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4501:
URL: https://github.com/apache/iggy/pull/4501#issuecomment-6130333143
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/4501?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `92.68293%` with `3 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 78.99%. Comparing base ([`8ded547`](https://app.codecov.io/gh/apache/iggy/commit/8ded54789a0ac08414de4f73d7311789856ba2c6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ea75055`](https://app.codecov.io/gh/apache/iggy/commit/ea750550bb34db5f5cd1e1ebcde0de8aa764aff5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4501?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [core/sdk/src/tcp/tcp\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Ftcp%2Ftcp_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3RjcC90Y3BfY2xpZW50LnJz) | 90.47% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/sdk/src/diagnostic\_events.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fdiagnostic_events.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL2RpYWdub3N0aWNfZXZlbnRzLnJz) | 94.11% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #4501 +/- ##
============================================
- Coverage 88.17% 78.99% -9.18%
+ Complexity 1726 1725 -1
============================================
Files 1314 1267 -47
Lines 264065 190526 -73539
Branches 222719 149161 -73558
============================================
- Hits 232842 150510 -82332
- Misses 25732 34360 +8628
- Partials 5491 5656 +165
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.82% <92.68%> (-11.05%)` | :arrow_down: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.11% <ø> (-0.08%)` | :arrow_down: |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.39% <ø> (-1.02%)` | :arrow_down: |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.43% <ø> (+0.04%)` | :arrow_up: |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.84% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `95.14% <ø> (ø)` | |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.39% <ø> (ø)` | |
| [C++ SDK](https://app.codecov.io/gh/apache/iggy/pull/4501/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.43% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4501?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/sdk/src/http/http\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fhttp%2Fhttp_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL2h0dHAvaHR0cF9jbGllbnQucnM=) | `92.79% <100.00%> (ø)` | |
| [core/sdk/src/quic/quic\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fquic%2Fquic_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3F1aWMvcXVpY19jbGllbnQucnM=) | `85.91% <100.00%> (+0.22%)` | :arrow_up: |
| [core/sdk/src/websocket/websocket\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fwebsocket%2Fwebsocket_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3dlYnNvY2tldC93ZWJzb2NrZXRfY2xpZW50LnJz) | `86.03% <100.00%> (-0.42%)` | :arrow_down: |
| [core/sdk/src/diagnostic\_events.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fdiagnostic_events.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL2RpYWdub3N0aWNfZXZlbnRzLnJz) | `94.11% <94.11%> (ø)` | |
| [core/sdk/src/tcp/tcp\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/4501?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Ftcp%2Ftcp_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3RjcC90Y3BfY2xpZW50LnJz) | `92.64% <90.47%> (-0.19%)` | :arrow_down: |
... and [421 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/4501/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "haubur (via GitHub)" <gi...@apache.org>.
haubur commented on code in PR #4501:
URL: https://github.com/apache/iggy/pull/4501#discussion_r4251832361
##########
core/sdk/src/http/http_client.rs:
##########
@@ -242,7 +243,7 @@ impl HttpClient {
heartbeat_interval: config.heartbeat_interval,
access_token: IggyRwLock::new(access_token),
send_contexts: ConsumerGroupClientState::new(),
- events: broadcast(1000),
+ events: diagnostic_event_channel(),
Review Comment:
Maybe we should document that this is dead.
HTTP is stateless. The `HttpClient` implements the Client trait which has `subscribe_events` has a required method. Thats why it exists here but `subscribe_events` is (correctly) not implemented.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4501:
URL: https://github.com/apache/iggy/pull/4501#issuecomment-6129904133
<!-- iggy-pr-triage-welcome -->
Thanks for the PR. It is labeled `S-waiting-on-review` and queued for review.
Slash commands (own line, regular comment) move it around the queue:
- `/ready` - back to `S-waiting-on-review` after addressing feedback
- `/author` - flip to `S-waiting-on-author` while you finish changes
- `/request-review @user-or-team` - request a reviewer
- `/pin` - exempt the PR from the stale bot, `/unpin` to undo
See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md#pr-triage-commands) for details.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "TomPlanche (via GitHub)" <gi...@apache.org>.
TomPlanche commented on PR #4501:
URL: https://github.com/apache/iggy/pull/4501#issuecomment-6141221032
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sdk): drop the oldest diagnostic event instead of blocking [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #4501:
URL: https://github.com/apache/iggy/pull/4501#discussion_r4255538939
##########
core/common/src/traits/client.rs:
##########
@@ -67,5 +67,10 @@ pub trait Client:
async fn shutdown(&self) -> Result<(), IggyError>;
/// Subscribe to diagnostic events.
+ ///
+ /// The event channel is bounded. When it is full, the client drops the
+ /// oldest event, so publishing never waits for a subscriber. A subscriber
+ /// that falls behind loses old events, not the newest ones. HTTP keeps no
Review Comment:
warning: these docs omit that `recv()` returns `RecvError::Overflowed` before retained events. document that callers must retry `recv()` after overflow or use `StreamExt::next()`.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org