atiaomar1978-hub commented on PR #26028: URL: https://github.com/apache/camel/pull/26028#issuecomment-5504658411
## PR review summary _AI-generated review on behalf of atiaomar1978-hub_ **Verdict: Approve in principle** — this is a well-scoped, production-motivated fix with strong tests. The two-commit split (camel-support first, then camel-master) is the right dependency order. ### What works well - **Root cause addressed directly:** Leadership-lost during the 1s pending start window is no longer dropped; the scheduled task is cancelled and `leadershipTaken` is re-checked before starting the delegated consumer. - **CAMEL-24584 is a necessary foundation:** `BackgroundTask` auto-unschedule makes `isStartPending()` meaningful and stops lifetime no-op rescheduling. - **Failed-start recovery:** Publishing `delegatedConsumer` only after a successful `startService()` fixes the “stuck leader that never retries” failure mode — covered by `testConsumerStartsAfterLeadershipIsTakenAgainWhenAnEarlierStartFailed`. - **Operational visibility:** ERROR log when backoff is exhausted while still holding leadership is valuable for on-call. - **Tests:** `MasterConsumerLeadershipTest` is an excellent reproducer harness; the PR author’s note about verifying failures on unpatched `main` adds confidence. ### Inline notes (non-blocking follow-ups) 1. **Partial attempt cleanup** — if `createConsumer()` succeeded but leadership is lost before publish, `attempt` may hold an unstarted consumer that is not shut down (see inline on `MasterConsumer`). 2. **Upgrade guide** — consider a `camel-master` entry alongside the `camel-support` note for operators who hit the K8s/Pub/Sub follower-consumer symptom. 3. **Pre-existing lock ordering** — agree with the PR description that cluster-view ↔ consumer lock inversion remains a separate follow-up. ### Testing / CI CI had not reported checks on the branch at review time. Locally the described matrix (7 new master tests + 2 BackgroundTask tests) is appropriate for this change set. Thanks @henrik242 for the thorough PR description and the transparent “known pre-existing issues” section — that made review much easier. -- 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]
