UNOMI-954: Make Camel route refresh resilient to transient failures#779
Merged
Conversation
The snap-and-clear in consumeConfigsToBeRefresh() removed a config from the queue before its route was built, so any failure permanently dropped it with no retry. Fix: - Add requeueForRefresh() to ImportExportConfigurationService so the timer can re-schedule a failed config for the next tick, with a cap of 5 attempts before giving up. - updateProfileImportReaderRoute() now throws instead of silently returning when the config cannot be loaded, allowing the catch block to trigger the re-queue. - Add initialDelay=0 to the Camel file endpoint so the route polls immediately on start instead of waiting the default 1 second. - Harden ProfileImportActorsIT: gate on route start via keepTrying, force refreshIndex(Profile.class) before each ES search attempt.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves reliability of the router’s Camel route refresh mechanism by re-queuing failed refresh operations (instead of dropping them after consumeConfigsToBeRefresh() clears the queue), and hardens an integration test to wait for route startup and reduce Elasticsearch visibility flakiness.
Changes:
- Add
requeueForRefresh(...)toImportExportConfigurationServiceand implement it for import/export configuration services to support retry-on-next-tick behavior. - Add retry logic in
RouterCamelContextfor failed route refresh operations (with a max retry cap) and make import route refresh throw when a config cannot be loaded. - Update
ProfileImportActorsITto wait for Camel route start and refresh the ES index before polling for imported profiles.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| itests/src/test/java/org/apache/unomi/itests/ProfileImportActorsIT.java | Gates test progress on route startup and refreshes ES index before polling results. |
| extensions/router/router-service/src/main/java/org/apache/unomi/router/services/ImportConfigurationServiceImpl.java | Implements requeueForRefresh to re-add failed import configs to the refresh queue. |
| extensions/router/router-service/src/main/java/org/apache/unomi/router/services/ExportConfigurationServiceImpl.java | Implements requeueForRefresh to re-add failed export configs to the refresh queue. |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java | Adjusts file endpoint options to poll immediately on route start. |
| extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java | Adds retry tracking/capping and re-queuing on refresh failures; makes import refresh throw on missing config. |
| extensions/router/router-api/src/main/java/org/apache/unomi/router/api/services/ImportExportConfigurationService.java | Adds API for re-queuing configs for refresh retries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…missing export configs Guard against blank or schemeless source endpoints in ProfileImportFromSourceRouteBuilder, which previously crashed with an NPE or StringIndexOutOfBoundsException instead of skipping the route. Make updateProfileExportReaderRoute() throw on a missing config like its import counterpart already does, so the export refresh loop correctly triggers the retry/requeue logic instead of silently giving up. Also namespace the route-creation retry counter by direction and tenant to avoid key collisions between import/export configs and across tenants.
…ive-up Replace the colon-delimited String retry key with a RetryKey record so tenant/config IDs can never collide across import/export or tenants. Persist CONFIG_STATUS_ROUTE_CREATION_FAILED on the configuration once retries are exhausted, instead of only logging it, so the failure is visible through the existing config APIs rather than only in logs.
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
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.
Summary
Fixes a reliability gap where saving an import/export configuration could silently fail to create the corresponding Camel route, leaving the import/export permanently inactive with no error shown to the user.
requeueForRefresh()toImportExportConfigurationServiceso the timer re-schedules a failed config for the next tick, capped at 5 attemptsupdateProfileImportReaderRoute()now throws instead of silently returning when the config cannot be loaded, allowing the catch block to trigger the re-queueinitialDelay=0to the Camel file endpoint so the route polls immediately on start instead of waiting the default 1 secondProfileImportActorsIT: gate on route start viakeepTrying, forcerefreshIndex(Profile.class)before each ES search attemptTest plan
ProfileImportActorsIT,ProfileImportSurfersIT,ProfileExportIT)🤖 Generated with Claude Code