unbridled-41 opened a new pull request, #4243:
URL: https://github.com/apache/rocketmq-dashboard/pull/4243
Fixes #4242.
## Problem / Evidence
Acknowledging a REMINDER system alert marks the event acknowledged but never
ACKs the underlying alert state, so the reminder loop keeps producing new
events and notification sends after the user acknowledged the alert.
Evidence chain (all verified in the current base):
1. REMINDER events are acknowledgeable in the shipped UI —
`web/src/pages/ops/systemAlerts.tsx` renders the Acknowledge button for every
row where `!alert.acknowledged && alert.transition !== 'RESOLVED'`, and the
REMINDER row is the newest row for any alert firing longer than one reminder
interval (default 30m).
2. `AlertService.acknowledgeAlert` gates the state update on
`"FIRING".equalsIgnoreCase(alert.getTransition())` — REMINDER events skip
`alertStateRepository.acknowledge` entirely.
3. Even with the whitelist widened, `RmqAlertStateMapper.acknowledgeFiring`
requires `AND fired_at = #{firedAt}`; a REMINDER event's `time` is the reminder
time (strictly after the current episode's `fired_at`), so the equality can
never match.
4. The loop is real: `AlertStateMachine.advanceHit` emits REMINDER every
reminder interval while the state is `FIRING`, and
`NativeAlertProcessor`/`NativeAlertEvaluationService` enqueue a notification
for both FIRING and REMINDER transitions. Only `ACKED` stops it (`advanceHit`
returns NONE while ACKED).
Reproduced by the new service-level regression test: acknowledging an event
with `transition("REMINDER")` invoked `alertStateRepository.acknowledge` zero
times before the fix (red), and by the new H2 integration test:
`acknowledgeFiring(ruleId, fingerprint, firedAt + 30min, now)` updated 0 rows
before the fix (`expected: 1 but was: 0`).
## Root cause / Fix
Two coupled conditions excluded reminder events from the state transition
they are part of:
1. `AlertService.acknowledgeAlert` now accepts `FIRING` or `REMINDER`
transitions (both belong to the same firing episode).
2. The mapper predicate changes from `fired_at = #{firedAt}` to `fired_at <=
#{firedAt}`.
The relaxed predicate preserves the stale-episode protection that
`acknowledgingResolvedEventMustNotAcknowledgeANewerFiringStateTest` pins: all
events of the current episode have `time >= fired_at` (the FIRING event's time
equals `fired_at`, reminders are later), while every event of a previous
episode precedes the current episode's `fired_at` (the condition must clear and
re-accumulate PENDING before a new episode fires), so `fired_at <= event.time`
matches exactly the current episode.
## Priority & scoring
PRIORITY 72 = impact 32 (the acknowledge action silently fails at its only
purpose — stopping paging — for every alert firing longer than one reminder
interval) + blast radius 12 (all native alert rules with reminders enabled,
both API and UI paths) + reproducibility 18 (deterministic state machine
transition; verified red→green by unit and H2 tests) + maintenance value 10
(completes the FIRING/REMINDER lifecycle contract the state machine already
defines). FIX_CONFIDENCE 85: two-line condition widening plus a mapper
predicate whose stale-episode safety is proven by both new tests and the
untouched existing guard test.
## Tests
- New service regression
`acknowledgingReminderEventShouldAcknowledgeItsActiveRuleStateTest`
(`AlertServiceTest`): red before (acknowledge never invoked), green after.
- New H2 integration test `RmqAlertStateMapperIntegrationTest` (2 tests):
reminder time of the current episode ACKs (`expected: 1 but was: 0` red
before); an event older than the current episode does not ACK and the row stays
FIRING (guard, green before and after).
- `mvn -o test
-Dtest='AlertServiceTest,RmqAlertStateMapperIntegrationTest,MybatisPlusAlertStateRepositoryTest,AlertStateMachineTest'`
→ all pass; `mvn -o test -Dtest='org.apache.rocketmq.studio.ops.alert.*Test'`
→ 33 alert test classes all pass (AlertServiceTest 77/77 incl. the pre-existing
FIRING and RESOLVED acknowledge tests unchanged).
- Full backend suite `mvn -o test` on this branch: **2154 tests, 2 failures
— both pre-existing baseline failures (`AuthCorsIntegrationTest` ×2, documented
in the repo's test baseline; the Aliyun baseline failure did not occur this
run). Zero new failures.**
- `git diff` self-check: 4 files, +114/−3 (2 source files, 2 test files), no
unrelated changes.
## Risk
Low. The only behavior change is that acknowledging a reminder event of the
current firing episode now ACKs the state (stopping further reminders) — the
action the UI already offers and operators already expect. RESOLVED events
remain inert (existing test unchanged), and events older than the current
episode still cannot ACK a newer firing state (new guard test). The atomicity
of the two writes in `acknowledgeAlert` is a separate, already-reported concern
(#4203) and is intentionally not touched here.
--
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]
