fix(orchestrator): requeue SNOS jobs when RPC is unavailable #897
+199
−2
Conversation
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
…of failing - Add check_ready_to_process method to JobHandlerTrait for dependency checks - Implement SNOS health check using chain_id RPC call - Requeue jobs with delay when SNOS RPC is down instead of failing them - Send single alert per downtime period to avoid alert fatigue - Add tests for health check and requeue behavior
heemankv
requested changes
Dec 10, 2025
| } | ||
|
|
||
| async fn check_ready_to_process(&self, config: Arc<Config>) -> Result<(), Duration> { | ||
| let snos_url = config.snos_config().rpc_for_snos.to_string(); |
Contributor
There was a problem hiding this comment.
rpc_for_snos is already a Url, any reason to re-parse it ?
Comment on lines
+240
to
+246
| if !SNOS_UNAVAILABLE_ALERT_SENT.swap(true, Ordering::SeqCst) { | ||
| let alert_msg = | ||
| format!("SNOS RPC {} is unavailable. SnosRun jobs will be requeued until it recovers.", snos_url); | ||
| if let Err(e) = config.alerts().send_message(alert_msg).await { | ||
| error!(error = ?e, "Failed to send SNOS unavailability alert"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
If config.alerts().send_message(alert_msg) fails, the flag is already set to true, so the alert will never be attempted again (even if SNOS recovers and goes down again later) !
heemankv
requested changes
Dec 10, 2025
Comment on lines
+240
to
+246
| if !SNOS_UNAVAILABLE_ALERT_SENT.swap(true, Ordering::SeqCst) { | ||
| let alert_msg = | ||
| format!("SNOS RPC {} is unavailable. SnosRun jobs will be requeued until it recovers.", snos_url); | ||
| if let Err(e) = config.alerts().send_message(alert_msg).await { | ||
| error!(error = ?e, "Failed to send SNOS unavailability alert"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Also this will only solve alert aggregation properly only when we have 1 pod of orchestrator running.
Over on multiple pods the aggregation reduce inversely with number of pods
Contributor
There was a problem hiding this comment.
Should we just handle aggregation outside of orchestrator ?
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.
Pull Request type
What is the current behavior?
When the SNOS RPC endpoint (Pathfinder) is down, all SnosRun jobs fail one by one, sending alerts for each failure. This causes:
Resolves: #NA
What is the new behavior?
check_ready_to_process()method toJobHandlerTraitthat allows handlers to verify dependencies are available before job processing beginsSnosJobHandlerimplements this method to check if SNOS RPC is reachable usingchain_id()RPC callAtomicBoolto send only one alert when SNOS goes down, preventing alert fatigueFlow:
Does this introduce a breaking change?
No - This is an additive change. The
check_ready_to_process()method has a default implementation returningOk(()), so existing job handlers continue to work without modification.Other information
Files changed:
orchestrator/src/worker/event_handler/jobs/mod.rs- Addedcheck_ready_to_processto traitorchestrator/src/worker/event_handler/jobs/snos.rs- Implemented health check for SNOSorchestrator/src/worker/event_handler/service.rs- Call check before processingorchestrator/src/worker/service.rs- Madeadd_job_to_queuepub(crate)orchestrator/src/tests/jobs/snos_job/mod.rs- Added health check testsorchestrator/src/tests/jobs/mod.rs- Added requeue behavior test