allthingssecurity commented on PR #27311: URL: https://github.com/apache/camel/pull/27311#issuecomment-5966441099
Thanks for the review. Addressed in fb1cc4d50138: - `onFailure` no longer releases the lock. It only steps down locally (`setMaster(false)`) and schedules the re-watch after `sessionRefreshInterval` (at least 1s). The hand-over is left to the session TTL plus `lock-delay`. When the next query shows the session still holds the key, the node becomes leader again. - The executor is now created after `createSession()`. If acquiring the lock or starting the watch fails afterwards, it is shut down before the exception is rethrown. - Upgrade guide: this PR never added a note there, so nothing to remove. With the lock kept, it changes no defaults or options, so no note is needed. Tests: `ConsulClusterViewRecoveryTest` now has 5 tests. `releasesTheLockOfItsPathAfterAFailedQuery` became `keepsTheLockAfterAFailedQuery`. It checks that the node steps down, `releaseLock` is never called, the session still holds the key, another session cannot acquire it, and the node is leader again on the next answer. The new test `doesNotLeaveAnExecutorBehindWhenTheSessionCannotBeCreated` makes `createSession()` fail during start and verifies no scheduled executor was created. Both new tests fail against the previous code. `mvn -pl components/camel-consul install`: 9 unit tests pass, 0 failures. The 27 ITs are skipped locally because they need Docker. _Claude Code on behalf of allthingssecurity_ -- 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]
