Add exponential backoff retry for retryable SQLite loAdd exponential backoff retry for retryable SQLite lock errorsck errors - #39
Conversation
When fetch_orchestration_item encounters SQLITE_BUSY errors due to concurrent INSERT/SELECT operations, the orchestration dispatcher now implements exponential backoff retry logic instead of treating all errors uniformly. Changes: - Track consecutive retryable errors per worker - Implement exponential backoff: 200ms, 400ms, 800ms, 1600ms, 3200ms (max 5s) - Reset counter on successful fetch or permanent errors - Enhanced error logging with attempt number and backoff duration This resolves race conditions during high-concurrency startup where start_orchestration() INSERTs and dispatcher SELECTs conflict, causing workflows to fail with 'database is locked' errors even with PRAGMA busy_timeout configured. Tested with concurrent workflow submission - first attempt hit lock, second attempt (after 200ms backoff) succeeded, workflow executed and completed successfully.
|
Thanks @Wolfman56. Would you pls mind doing the same for the work item dispatcher as well for completeness and symmetry? |
|
I'll have my agent go back and do that! Best. |
Implements the same exponential backoff logic from the orchestration dispatcher to the worker dispatcher, addressing @affandar's request for completeness and symmetry. Changes: - Track consecutive retryable errors per worker - Exponential backoff: 200ms, 400ms, 800ms, 1600ms, 3200ms (max 5s) - Reset counter on successful fetch or permanent errors - Refactored process_next_work_item() to return (work_found, reset_counter) - Enhanced error logging with attempt number and backoff duration This ensures both dispatchers handle SQLite lock errors consistently during high-concurrency scenarios.
|
Ok, dispatcher updated as well |
|
Thanks for the update. Two suggestions below: 1. Simplify the cap logiclet backoff_ms = std::cmp::min(
100 * (2_u64.pow(consecutive_retryable_errors.min(5))),
5000 // ← Dead code
);The exponent is capped at 5, so the effective max is let backoff_ms = (100 * 2_u64.pow(consecutive_retryable_errors)).min(3000);2. Worker dispatcher: backoff should be in the main loopIn Simpler approach—return async fn process_next_work_item(...) -> Result<bool, ProviderError> {
match fetch_result {
Ok(Some(...)) => { /* process */; Ok(true) }
Ok(None) => Ok(false),
Err(e) => Err(e), // Let caller handle
}
}
// In loop:
match process_next_work_item(...).await {
Ok(found) => {
backoff_errors = 0;
work_found = found;
}
Err(e) if e.is_retryable() => {
backoff_errors += 1;
let delay_ms = (100 * 2_u64.pow(backoff_errors)).min(3000);
warn!(attempt = backoff_errors, delay_ms, error = ?e, "Retryable fetch error");
tokio::time::sleep(Duration::from_millis(delay_ms)).await;
continue;
}
Err(e) => {
warn!(error = ?e, "Permanent fetch error");
backoff_errors = 0;
tokio::time::sleep(Duration::from_millis(100)).await;
continue;
}
} |
|
merged, will implement the recommendation I had on top of it in a bit. |
|
Excellent. Sorry got distracted by day job. Keep up the great work. |
Problem
When
fetch_orchestration_item()encounters SQLITE_BUSY errors due to concurrent INSERT/SELECT operations during high-concurrency startup, workflows fail with "database is locked" errors even thoughPRAGMA busy_timeout = 60000is configured. This race condition occurs whenstart_orchestration()INSERTs and the dispatcher tries to SELECT simultaneously.Solution
Implemented exponential backoff retry logic in the orchestration dispatcher specifically for retryable errors (database locks, connection errors).
Changes
Testing
Tested with concurrent workflow submission scenario:
Error fetching orchestration item (retryable, attempt 1)...backing off 200msImpact
This fix resolves the race condition without requiring changes to SQLite pragma configuration or connection pool settings.