unbridled-41 opened a new pull request, #4566:
URL: https://github.com/apache/rocketmq-dashboard/pull/4566
Fixes #4565.
## Problem / Evidence
The Apache LiteTopic **list** and **quota** pages fail completely with HTTP
502 when a single broker master fails its `getBrokerLiteInfo` call.
`discoverParentTopics` (`RocketMQLiteTopicProvider.java:159` on master
`d50ffecc`) and the `getQuota` master loop (:394) call
`admin.getBrokerLiteInfo(master)` without the per-master try/catch that guards
every other per-master read in the same file (`getParentTopicInfo`,
`getLiteGroupInfo`, `getLiteClientInfo`, `examineConsumerConnectionInfo`,
`getBrokerConfig`, `examineTopicConfig` all catch and continue). Any exception
escapes the admin action, which `MqAdminExtFactory.execute` converts into
`BusinessException(502, "RocketMQ admin call failed: …")`.
Reachability: a rolling upgrade where a non-first master runs a pre-lite
build (unsupported-code `MQBrokerException`), or one master restarting /
unreachable from the dashboard while its peers answer. `isSupported()` probes
only `masters.get(0)`, so on a mixed cluster the capability gate passes and the
console walks straight into the 502.
Red (this PR's new regression, master + tests): two masters,
`getBrokerLiteInfo(m2)` throws — `listLiteTopics` and `getQuota` both propagate
the failure (`IllegalStateException: broker restarting` escapes the action)
although m1 reported a fully usable parent topic and quota counts.
## Root cause / Fix
Add the same per-master degradation the sibling reads already apply: catch
per master, warn-log `"Skipping master {} for the LiteTopic list/quota"`, and
aggregate the masters that answered. The existing empty-list/503 no-master
semantics are unchanged. `isSupported`'s own single-master probe is a separate
observation and deliberately untouched here (tracked separately to keep this
change minimal).
- `RocketMQLiteTopicProvider.java`: +19/−2 (two try/catch wrappers around
`getBrokerLiteInfo` in `discoverParentTopics` and `getQuota`).
- `RocketMQLiteTopicProviderTest.java`: +44/−0
(`listLiteTopicsSkipsAMasterWhoseLiteInfoFailsInsteadOfFailingThePage`,
`quotaSkipsAMasterWhoseLiteInfoFailsInsteadOfFailingThePage`).
## Priority & scoring
PRIORITY 74 = impact 30 (one degraded broker takes down the whole LiteTopic
inventory + quota cards cluster-wide; a rolling upgrade triggers it
deterministically) + scope 12 (single provider, list+quota paths) +
reproducibility 18 (two-master mock reproduces exactly; trigger conditions are
ordinary ops events) + maintenance value 14 (aligns the file with the
per-master grading its sibling providers and its own helper methods already
follow). FIX_CONFIDENCE 96: mirrors an in-file degradation pattern, no API
change, two deterministic regressions.
## Tests
- `mvn -o test -Dtest=RocketMQLiteTopicProviderTest`
- Red (master + new tests): `Tests run: 17, Failures: 1, Errors: 2` — both
new tests errored with the escaping `IllegalStateException` (list + quota) and,
in the same run before scoping, the capability-probe twin failed separately;
after scoping this PR keeps only the two list/quota regressions, re-run:
**16/16 passed, 0 failures**.
- Green on the final branch state: `Tests run: 16, Failures: 0, Errors: 0`
plus `LiteTopicServiceTest` 10/10 and `LiteTopicControllerTest` 8/8; `mvn -o
checkstyle:check` → 0 violations.
- Full backend suite (`SPRING_PROFILES_ACTIVE=dev mvn -o clean test`, this
exact tree): **2416 tests, 0 failures** (`Tests run: 2416, Failures: 0, Errors:
0, Skipped: 0` — BUILD SUCCESS). Pristine `origin/master` `d50ffecc` measured
on the identical XML basis in a clean worktree: **2414 tests, 0 failures**; the
+2 delta is exactly the two new regression tests.
## Risk
Low. Degradation-only: a failing master's contribution is skipped with a
warn log instead of failing the request; no API or response-shape change. If
**all** masters fail, the behavior stays as today (empty result / existing
no-master error paths), so no silent-success surface is added.
Upstream CI note: the repository's CI workflow fails at startup for every
branch and PR (infrastructure-level, also on maintainer branches), so local
verification above substitutes for it.
--
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]
