You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2026/03/23 11:43:10 UTC
[PR] [fix][client] Fix stale Healthy state in SameAuthParamsLookupAutoClusterFailover causing flaky test [pulsar]
lhotari opened a new pull request, #25388:
URL: https://github.com/apache/pulsar/pull/25388
### Motivation
The `SameAuthParamsLookupAutoClusterFailoverTest.testAutoClusterFailover` integration test is flaky. The root cause is a bug in `SameAuthParamsLookupAutoClusterFailover.findFailoverTo()`: when it probes intermediate services and finds them unavailable, it does not update their state. They remain stale `Healthy`, which causes a spurious recovery bounce on the next check cycle.
For example, when failing over from index 0 to index 2 (skipping index 1 which is unavailable), index 1's state remains `Healthy`. On the next check, `firstHealthyPulsarService()` sees the stale `Healthy` state and immediately "recovers" to index 1 — which is actually a broken service. This causes unnecessary bouncing (0→2→1→2 instead of 0→2). Combined with 3-second probe timeouts on dead services, the extra bounce adds ~30 seconds, pushing the total failover time past the test's 60-second awaitility timeout.
### Modifications
- In `findFailoverTo()`, when a probe fails for a service, mark it as `Failed` immediately. This prevents stale `Healthy` states from causing spurious recovery bounces after failover.
- Added unit tests in `pulsar-client` to verify the fix and prevent regression.
### Verifying this change
This change added tests and can be verified as follows:
- `testFindFailoverToMarksSkippedServicesAsFailed` — directly verifies that `findFailoverTo` marks skipped unavailable services as `Failed` (the core bug). Verified this test fails without the fix.
- `testNoSpuriousRecoveryBounceAfterFailover` — verifies no spurious recovery bounce occurs on the next check cycle after failover.
- `testRecoveryAfterFindFailoverToMarksServiceFailed` — verifies that recovery still works correctly after a service was marked `Failed` by `findFailoverTo`, once it becomes available again.
### Does this pull request potentially affect one of the following parts:
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
### Documentation
- [x] `doc-not-needed`
### Matching PR in forked repository
PR in forked repository: <\!-- ENTER URL HERE after creating forked PR -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Fix stale Healthy state in SameAuthParamsLookupAutoClusterFailover causing flaky test [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25388:
URL: https://github.com/apache/pulsar/pull/25388
--
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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][client] Fix stale Healthy state in SameAuthParamsLookupAutoClusterFailover causing flaky test [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25388:
URL: https://github.com/apache/pulsar/pull/25388#issuecomment-4110432376
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/25388?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 72.76%. Comparing base ([`953d092`](https://app.codecov.io/gh/apache/pulsar/commit/953d092b10ab7ae5215677aeb46df2a5df85c2eb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`189d32b`](https://app.codecov.io/gh/apache/pulsar/commit/189d32b241f6e36ab1e7db16598e7eee6d1011ef?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/25388?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #25388 +/- ##
============================================
+ Coverage 72.21% 72.76% +0.54%
- Complexity 34016 34289 +273
============================================
Files 1954 1954
Lines 154857 154859 +2
Branches 17739 17740 +1
============================================
+ Hits 111837 112689 +852
+ Misses 33959 33110 -849
+ Partials 9061 9060 -1
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/25388/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/25388/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `25.77% <0.00%> (-0.04%)` | :arrow_down: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/25388/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `22.50% <0.00%> (+<0.01%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/25388/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.74% <100.00%> (+0.60%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/25388?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../impl/SameAuthParamsLookupAutoClusterFailover.java](https://app.codecov.io/gh/apache/pulsar/pull/25388?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FSameAuthParamsLookupAutoClusterFailover.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1NhbWVBdXRoUGFyYW1zTG9va3VwQXV0b0NsdXN0ZXJGYWlsb3Zlci5qYXZh) | `68.04% <100.00%> (+1.58%)` | :arrow_up: |
... and [138 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/25388/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@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org