You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/10/09 19:21:06 UTC
[PR] fix(alert): keep the bulk delivery retry going when one id fails unexpectedly [rocketmq-dashboard]
unbridled-41 opened a new pull request, #5938:
URL: https://github.com/apache/rocketmq-dashboard/pull/5938
### Which Issue(s) This PR Fixes
Fixes #5937
### Problem / Evidence
The loop's stated purpose is per-id isolation, but it only catches `BusinessException`:
```java
// NotificationOutboxService.retryFailedDeliveries (before)
try { retryFailedDelivery(deliveryId); succeeded.add(deliveryId); }
catch (BusinessException error) { failures.put(deliveryId, error.getMessage()); }
```
Each id is committed by its own `mapper.update` before the next iteration, so a `RuntimeException` on a later row escapes and 500s the request while earlier rows are already `PENDING`:
```
NotificationOutboxServiceTest#bulkRetryKeepsTheOutcomeOfEarlierDeliveriesWhenALaterOneFailsUnexpectedlyTest
org.springframework.dao.DataAccessResourceFailureException: db down (escapes the call)
```
The page shows the bulk-retry error and does not reload, so those rows keep displaying `FAILED` until the next refresh although the dispatcher will deliver them.
### Root cause / Fix
Unexpected failures were treated as fatal for the whole request. Catch `RuntimeException` per item as well, record it beside the business failures (message, or the exception class when it carries none), and keep the loop's guarantee - the pattern `InstanceService.deleteInstances` and `AlertService.bulkDeleteRules` already use.
### Priority and scoring
**PRIORITY 48** — impact 16/40 (a partially applied action reported as a total failure; the table then contradicts the outbox), blast radius 10/20 (bulk retry after an infrastructure hiccup), reproducibility 18/20 (deterministic with one failing id, pinned by the new test), maintenance value 4/20.
**FIX_CONFIDENCE 85** — one catch clause, with two precedents in the same codebase.
### Tests
`cd server && mvn -o -B -ntp test -Dtest='org.apache.rocketmq.studio.ops.alert.**'` -> `Tests run: 327, Failures: 0, Errors: 0`.
| Test | Before | After |
|---|---|---|
| `bulkRetryKeepsTheOutcomeOfEarlierDeliveriesWhenALaterOneFailsUnexpectedlyTest` | FAIL (the exception escapes the call) | PASS (`succeededIds=[8]`, `failures` contains 9) |
`mvn -o -B -ntp checkstyle:check` passes.
### Risk
Only the failure bookkeeping changes: a request that used to fail wholesale now reports which ids succeeded, which is what the response type already models.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(alert): keep the bulk delivery retry going when one id fails unexpectedly [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5938: fix(alert): keep the bulk delivery retry going when one id fails unexpectedly
URL: https://github.com/apache/rocketmq-dashboard/pull/5938
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(alert): keep the bulk delivery retry going when one id fails unexpectedly [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5938:
URL: https://github.com/apache/rocketmq-dashboard/pull/5938#issuecomment-6091342544
### Verification on the final commit (branch + commit + command results)
Every command from the description re-run on a fresh worktree of this branch (`git worktree add <dir> fork/<branch>`, no local modifications), at commit `d2daf2fae9`:
| command | exit | result |
| --- | --- | --- |
| `cd server && mvn -o -B -ntp test -Dtest='org.apache.rocketmq.studio.ops.alert.**'` | 0 | `Tests run: 327, Failures: 0, Errors: 0, Skipped: 0 · BUILD SUCCESS` |
CI note: on this repository the `Backend Build` (binary-license-gate), `Frontend Build` / `Frontend Docker Build` (licence manifest) and `rmqctl Build` (YAML renderer quoting) jobs are red for every branch, trunk included; the module-level commands above are unaffected.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(alert): keep the bulk delivery retry going when one id fails unexpectedly [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #5938:
URL: https://github.com/apache/rocketmq-dashboard/pull/5938#issuecomment-6087711184
### CI status of this PR
`Backend Tests (Java 21)` — still running at the time of writing; the change-specific commands and their results are in the Tests section of the description.
The four remaining red jobs are red for every branch, including `rocketmq-studio` itself:
- `Backend Build (Java 21)` — the `binary-license-gate` antrun step fails on a list of third-party jars (`spring-ai-*`, `swagger-ui`, `tencentcloud-sdk-*`, `zstd-jni`, ... : "missing complete upstream LICENSE/COPYING; a POM name or link is not a substitute"). This PR touches no build configuration; tracked by #5341.
- `Frontend Build (Node 20)` / `Frontend Docker Build` — the frontend licence manifest check fails the same way on branches that do not touch `web/` at all; tracked by #5346.
- `rmqctl Build (Go)` — `cmd` `TestMessageQueryCompletenessWarningTest` yaml subtests: `got {"items":[{"size":"3",...}]}, want {"size":3,...}`, i.e. the YAML renderer writes numbers as quoted strings where the test expects integer literals. Reproduced locally with `cd rmqctl && go test ./cmd/` from a checkout whose `rmqctl/` tree is byte-identical to `rocketmq-studio`'s, and owned by open PR #5850 ("keep yaml numbers unquoted").
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(alert): keep the bulk delivery retry going when one id fails unexpectedly [rocketmq-dashboard]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5938:
URL: https://github.com/apache/rocketmq-dashboard/pull/5938#issuecomment-6305452916
This pull request has been automatically closed because there has been no activity for 7 days. If you would like to continue working on this, please reopen the PR with updated changes.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org