CAMEL-23906: Fix flaky DelayerWhileShutdownTest on JDK 25#24422
Conversation
Replace Phaser-based synchronization with a simpler CountDownLatch and use Awaitility for the mock endpoint assertion. The Phaser approach was timing-sensitive under JDK 25's thread scheduling. The new approach uses a latch to signal when the short-delay route starts processing and increases the long delay to 5000ms to ensure the context shutdown always interrupts it before it completes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 23 tested, 0 compile-only — current: 0 all testedMaveniverse Scalpel detected 23 affected modules (current approach: 0).
|
There was a problem hiding this comment.
Pull request overview
This PR updates DelayerWhileShutdownTest to reduce flakiness (observed on newer JDK scheduling) by simplifying route synchronization and making the mock assertion tolerant of timing variations.
Changes:
- Replace
Phaser-based synchronization with aCountDownLatchsignal. - Replace direct
assertMockEndpointsSatisfied()with an Awaitility-based assertion window. - Adjust the “long delay” timing to widen the shutdown/interruption margin.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Wait until the short-delay route has entered the delay phase, | ||
| // ensuring both routes are actively processing | ||
| shortDelayStarted.await(5, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Fixed in ebe472b: the latch return value is now asserted with assertTrue() and a diagnostic message. Variable and comment also corrected.
| public void configure() { | ||
| from("seda:a").process(e -> phaser.arriveAndAwaitAdvance()).delay(1000).to("mock:result"); | ||
| from("seda:b").process(e -> phaser.arriveAndAwaitAdvance()).delay(1).to("mock:result"); | ||
| from("seda:a").delay(5000).to("mock:result"); |
There was a problem hiding this comment.
Fixed in ebe472b: the long delay is now 30000ms (30s), well above the 5s Awaitility assertion timeout, eliminating this race.
- Assert that the CountDownLatch returns true, failing fast with a diagnostic message if the short-delay route never starts processing - Increase the long delay from 5000ms to 30000ms so it is well above the 5-second Awaitility assertion timeout, eliminating the race where both could expire near the same instant under heavy CI load - Fix comment and rename variable for clarity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Phaser-based synchronization with a simplerCountDownLatchand use Awaitility for the mock endpoint assertionPhaserapproach was timing-sensitive under JDK 25's thread scheduling, causingtestSendingMessageGetsDelayedto fail intermittentlyWhat changed
The original test used a
Phaser(2)to synchronize both SEDA routes before they entered their delay phases, then immediately calledassertMockEndpointsSatisfied(). On JDK 25, different thread scheduling could cause the short-delay message (1ms) to not have arrived atmock:resultby the time of assertion.The fix:
PhaserwithCountDownLatch— simpler synchronization; the latch signals when the short-delay route begins processing, with an assertion on the return value for fast failure if the route never startsAwaitility.await()— replaces the directassertMockEndpointsSatisfied()call withawait().atMost(5, SECONDS).untilAsserted(...), making the assertion resilient to scheduling variationsTest plan
DelayerWhileShutdownTestpasses consistently (verified multiple consecutive runs)Claude Code on behalf of @oscerd