lizhimins commented on PR #3036:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3036#issuecomment-5537076145
Closing after detailed review. The underlying concern is partially real, but
this fix can't be taken:
1. The new javadoc claims notifications are retried "once the outbox
recovers", but no such re-enqueue path exists — once state is committed, an
enqueue failure means the notification is lost permanently.
2. It swallows programming errors like IllegalStateException("alert without
a persistent ID") as warnings.
3. It contradicts the rollback semantics established and tested in #2697
(NativeAlertEvaluationTransactionTest), and ships no tests.
Please consider redesigning (e.g., enqueue via the outbox within the same
commit unit, or explicit compensation) with test coverage, and resubmit. Thanks!
--
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]
