CAMEL-24860: a failed route reload restores the routes that ran before, instead of leaving the application without routes - #26641
Merged
Merged
Conversation
…e, without the failed file, instead of leaving the application without routes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
reviewed
Sep 21, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
The rollback logic is correct and the test covers the relevant scenario. Two issues in the implementation.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 563 of 692 tested, 26 compile-only — current: 563 all testedMaveniverse Scalpel detected 591 affected modules (current approach: 563). Skip-tests mode would test 563 modules (3 direct + 562 downstream), skip tests for 26 (generated code, meta-modules)
|
| Module | Duration | Status |
|---|---|---|
| Camel :: Docs | 20.9s | SUCCESS |
| Camel :: Support | 16.2s | SUCCESS |
| Camel :: YAML DSL | n/a |
Top 20 slowest modules:
Camel :: Docs(20.9s)Camel :: Support(16.2s)
…oad error it recovered from, and the Javadoc says the remembered set is cleared after a restore Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj
gnodet-bot
approved these changes
Sep 21, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Both findings from the previous review are addressed in 3a18dc2.
Unused cause parameter — fixed: cause.getMessage() is now used in the recovery WARN log line ("Reload failed due to: {}. The previous routes were restored …"), giving operators the load error message in context.
Inaccurate Javadoc — fixed: the sentence that said the previous set stays remembered has been replaced with the accurate description: after a successful restore previousSources is cleared and the next reload collects sources from the running routes.
The rollback logic itself is sound:
previousSourcesat the time the exception fires contains the attempted reload set (other routes + the failing file);restorePreviousRoutesfilters the failing file out byequalResourceLocationand reloads only the safe subset — correct.- The
getCamelContext().getRouteController().removeAllRoutes()/removeRouteTemplates/endpointRegistry.clear()fence beforeupdateRoutes(restore)correctly discards any partial state a failedupdateRoutesmay have left behind. - The guard
!removeAllRoutes || previousSources.isEmpty()is right: the incremental path never strips other routes, so no restore is needed there. previousSources.clear()after a successful restore is correct: the restored routes are the new ground truth; the next file-watcher tick will collect their sources itself.- The single-thread executor in
FileWatcherResourceReloadStrategymeans allpreviousSourcesmutations remain single-threaded — no new race introduced.
The Groovy test covers the essential scenario cleanly (failed save → other route stays Started, bad route gone; fixed save → both routes Started). The Spock given/when/then structure reads well.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When a route file is saved with a mistake in dev mode,
RouteWatcherReloadStrategy.onRouteReloadfirst stops and removes every route (removeAllRoutes), then loads the new set; when that load fails, the exception goes to the file watcher, which logs "Error reloading routes ... This exception is ignored", and the application runs with no routes at all until the next successful save. The strategy already remembers the previous sources for exactly that next save; nothing used them until then. In the stepwise benchmark a cron route was gone for 22 seconds after a bad save, and the only log lines were "Pausing trigger / Deleting job".The catch now restores the previous routes: the remembered sources minus the failed resources are reloaded (after clearing what a partial load may have left), a WARN says "Reload failed: the previous routes were restored (N route(s) running); the changed file loads on its next save", and the exception is rethrown as before, so the watcher's error line, the
CamelContextReloadFailureevent (CAMEL-24851) and the validator's report all still follow. The remembered set is cleared after the restore, since the running routes are the last working set again. If nothing can be restored, or the restore itself fails, a WARN says the application runs without routes until the file is fixed. Only theremoveAllRoutesmode is concerned; the incremental mode never removed the other routes.Pre-validating the file before removing the routes was considered:
preParseRoutecatches syntax but not the model errors the deserializers throw (pollEnrich: {uri: ...}, a cron endpoint without its name), and building into a throwaway context is heavy and has side effects. The rollback uses the machinery that exists and covers every kind of failure.Live check with
camel run --source-dir --devand two route files: the second saved with a mistake, the first kept logging through it (the WARN, the validator's report and the reload error in the log), and the fixed save brought both back. Upgrade guide note added.Tests
RouteReloadRollbackTest(camel-yaml-dsl, Spock, where a route file loader is on the test classpath): two YAML route files loaded, the strategy's reload callback driven by hand with a broken save of the second, the first route is Started again and the second is gone; the fixed save starts both.🤖 Generated with Claude Code
https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj