gnodet commented on code in PR #24819:
URL: https://github.com/apache/camel/pull/24819#discussion_r3602243876
##########
components/camel-quartz/src/test/java/org/apache/camel/component/quartz/QuartzAddDynamicRouteTest.java:
##########
@@ -39,12 +41,15 @@ public void testAddDynamicRoute() throws Exception {
context.addRoutes(new RouteBuilder() {
@Override
public void configure() {
-
from("quartz://myGroup/myTimerName?trigger.repeatInterval=2&trigger.repeatCount=1").routeId("myRoute")
+ // use a reasonable interval (200ms) so that the Quartz
+ // scheduler can reliably fire both triggers on loaded CI
+ // machines (2ms was far too tight for reliable scheduling)
+
from("quartz://myGroup/myTimerName?trigger.repeatInterval=200&trigger.repeatCount=1").routeId("myRoute")
.to("direct:foo");
}
});
- resultEndpoint.assertIsSatisfied();
+ MockEndpoint.assertIsSatisfied(context, 30, TimeUnit.SECONDS);
Review Comment:
_Claude Code on behalf of gnodet_
Thanks for the careful review! The 2ms interval was not originally chosen to
validate repeatCount enforcement — the original commit (a056a932, 2011) says
"Added test based on user forum issue" and the test's purpose is verifying that
dynamically adding a Quartz route to a running CamelContext works.
Neither interval validates "exactly N, no more" because
`MockEndpoint.expectedMessageCount(2)` returns as soon as 2 messages arrive
without waiting to detect extras — you would need `setAssertPeriod()` for that,
which neither the old nor new test uses.
The flakiness root cause is Quartz's misfire handling: `triggerStartDelay`
(default 500ms, see `QuartzEndpoint` line 96) delays fires to T+500ms and
T+502ms. On loaded CI, if the Quartz scheduler thread is delayed by just 3ms,
both fire times are in the past when the scheduler catches up.
`MISFIRE_INSTRUCTION_FIRE_NOW` (line 558) consolidates them into a single fire
— yielding 1 message instead of 2. Widening to 200ms gives the scheduler a
200ms window between fires, making it resilient to brief scheduler delays.
That said, if you think it would be valuable, we could add
`resultEndpoint.setAssertPeriod(500)` to verify no extra messages arrive after
the expected 2. This would address the repeatCount concern independently of the
interval change.
##########
components/camel-quartz/src/test/java/org/apache/camel/component/quartz/QuartzAddRoutesAfterCamelContextStartedTest.java:
##########
@@ -36,12 +38,15 @@ public void testAddRoutes() throws Exception {
context.addRoutes(new RouteBuilder() {
@Override
public void configure() {
-
from("quartz://myGroup/myTimerName?trigger.repeatInterval=100&trigger.repeatCount=1").to("mock:result");
+ // use a generous interval (500ms) so that the Quartz scheduler
+ // can reliably fire both the initial and repeat triggers even
+ // on heavily-loaded CI machines (100ms was too tight)
+
from("quartz://myGroup/myTimerName?trigger.repeatInterval=500&trigger.repeatCount=1").to("mock:result");
Review Comment:
_Claude Code on behalf of gnodet_
Good question! No message is actually dropped by Camel — the "lost" message
is caused by Quartz's built-in misfire handling, not the Camel runtime.
Here is the mechanism: `triggerStartDelay` (default 500ms, see
`QuartzEndpoint` line 96) ensures the endpoint is fully started before any
fire. With `repeatInterval=100ms` and `repeatCount=1`, fires are at T+500ms and
T+600ms. On loaded CI, if the Quartz scheduler thread is delayed by ~110ms (CPU
contention, GC), both fire times are in the past when the scheduler catches up.
Quartz's `MISFIRE_INSTRUCTION_FIRE_NOW` (`QuartzEndpoint` line 558)
consolidates these misfires into a single fire — by design. This is standard
Quartz behavior, not a Camel bug.
This test's interval has been adjusted before:
- 2010 (CAMEL-3203, original): 2ms
- 2013 (CAMEL-4075, camel-quartz2 port): increased to 1000ms
- 2021 (parallel tests refactor): reduced to 100ms for speed — which
introduced the flakiness
- This PR: increased to 500ms
The test's purpose (CAMEL-3203) is verifying that adding routes after
CamelContext started works, not stress-testing Quartz's sub-second scheduling.
The interval just needs to be large enough that the scheduler can reliably fire
both triggers without misfire consolidation.
--
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]
