diff --git a/bin/dipper-service/src/chain_client/client.rs b/bin/dipper-service/src/chain_client/client.rs index d25ad58a..5d8c374b 100644 --- a/bin/dipper-service/src/chain_client/client.rs +++ b/bin/dipper-service/src/chain_client/client.rs @@ -83,10 +83,12 @@ fn is_nonce_error(error: &str) -> bool { const MAX_NONCE_RETRIES: u32 = 2; /// How long one submission may hold `submit_lock` before giving up, derived from the retry -/// schedule so the cut-off can never pre-empt a retry the config allows: each nonce attempt -/// is at worst one full walk of the endpoint ring to read the nonce and another to send. +/// schedule so the cut-off can never pre-empt a retry the config allows: one full walk of +/// the endpoint ring to read the nonce and another to send. A second nonce attempt is not +/// budgeted separately: it only follows an endpoint answering with a nonce rejection, and +/// an endpoint that answers is not one that spends its whole retry schedule hanging. fn derive_submit_deadline(pool: &RpcProviderPool) -> Duration { - let budget = pool.worst_case_walk() * (2 * MAX_NONCE_RETRIES); + let budget = pool.worst_case_walk() * 2; // The rest of the worker job's budget stays reserved for what follows the broadcast: // the receipt poll and the nonce-gap fill. let cap = PROCESS_JOB_TIMEOUT / 5 * 4; @@ -1324,8 +1326,8 @@ mod tests { } /// The deadline exists to stop one submission starving the queue, not to cut off retries - /// the config asks for, so it is derived from the schedule: each nonce attempt walks the - /// whole ring twice, once reading the chain's nonce and once broadcasting. + /// the config asks for, so it is derived from the schedule: a submission walks the whole + /// ring twice, once reading the chain's nonce and once broadcasting. #[test] fn the_submit_deadline_covers_the_retry_schedule() { let client = client_over_retrying( @@ -1337,8 +1339,48 @@ mod tests { ); // Per endpoint: 2 attempts of 5s plus 1s of backoff; 2 endpoints make one walk of - // 22s; 2 walks for each of the 2 nonce attempts. - assert_eq!(client.inner.submit_deadline, Duration::from_secs(88)); + // 22s; one walk to read the nonce and one to send. + assert_eq!(client.inner.submit_deadline, Duration::from_secs(44)); + } + + /// The shape a real deployment has, 3 providers at the config defaults (10s timeout, + /// 3 retries), must fit under the cap with its whole schedule intact, otherwise every + /// production start would log the warning and lose retries the config asked for. + #[test] + fn three_providers_at_the_defaults_fit_inside_a_worker_job() { + let providers = (0..3) + .map(|i| { + format!("http://rpc{i}.invalid") + .parse() + .expect("provider URL") + }) + .collect(); + let config = ChainClientConfig { + enabled: true, + providers, + request_timeout: crate::config::default_chain_client_request_timeout(), + max_retries: crate::config::default_chain_client_max_retries(), + domain_refresh_interval: Duration::from_secs(3600), + gas_price_multiplier: 1.2, + max_gas_price_gwei: 100, + gas_buffer_multiplier: 2.0, + gas_floor: 100_000, + gas_max_addition: 200_000, + }; + let client = AlloyChainClient::new( + &config, + 1337, + Address::repeat_byte(0x11), + Address::repeat_byte(0x22), + &[0x42; 32], + ) + .expect("chain client"); + + // Per endpoint: 4 attempts of 10s plus 1+2+4s of backoff; 3 endpoints make one walk + // of 141s; two walks come to 282s, inside the 336s the job leaves for a submission. + let two_walks = Duration::from_secs(282); + assert_eq!(client.inner.submit_deadline, two_walks); + assert!(two_walks < PROCESS_JOB_TIMEOUT / 5 * 4); } /// A schedule that wants more time than a worker job has is capped rather than obeyed, diff --git a/bin/dipper-service/src/config.rs b/bin/dipper-service/src/config.rs index d9e0bad9..297bad3c 100644 --- a/bin/dipper-service/src/config.rs +++ b/bin/dipper-service/src/config.rs @@ -717,7 +717,7 @@ fn default_chain_client_enabled() -> bool { false } -fn default_chain_client_request_timeout() -> Duration { +pub(crate) fn default_chain_client_request_timeout() -> Duration { Duration::from_secs(10) } @@ -725,7 +725,7 @@ fn default_domain_refresh_interval() -> Duration { Duration::from_secs(3600) } -fn default_chain_client_max_retries() -> u32 { +pub(crate) fn default_chain_client_max_retries() -> u32 { 3 } diff --git a/bin/dipper-service/src/health.rs b/bin/dipper-service/src/health.rs index a86850ea..cef6f85e 100644 --- a/bin/dipper-service/src/health.rs +++ b/bin/dipper-service/src/health.rs @@ -14,9 +14,10 @@ use std::{ use axum::{Router, extract::State, http::StatusCode, response::IntoResponse, routing::get}; use tokio::{net::TcpListener, sync::mpsc}; -/// Default staleness threshold, twice the [`crate::worker::service::PROCESS_JOB_TIMEOUT`] of 300s +/// Default staleness threshold, twice the [`crate::worker::service::PROCESS_JOB_TIMEOUT`] /// that bounds a single job, so a legitimately slow job never trips the probe. -pub const DEFAULT_HEALTH_THRESHOLD: Duration = Duration::from_secs(600); +pub const DEFAULT_HEALTH_THRESHOLD: Duration = + Duration::from_secs(2 * crate::worker::service::PROCESS_JOB_TIMEOUT.as_secs()); /// Reference point for every watermark, fixed the first time liveness is touched. Watermarks are /// seconds since this instant rather than wall-clock stamps, so an NTP step cannot make a healthy @@ -183,7 +184,8 @@ mod tests { #[test] fn default_threshold_exceeds_the_job_timeout() { // A job timeout at or above the threshold means a job running to its bound looks wedged, - // so k8s restarts a healthy pod. Raising the timeout past 600s must fail here first. + // so k8s restarts a healthy pod. The threshold is derived from the timeout, so this + // guards a future edit that sets it by hand. assert!( DEFAULT_HEALTH_THRESHOLD > crate::worker::service::PROCESS_JOB_TIMEOUT, "the health threshold must leave room for one full-length job" diff --git a/bin/dipper-service/src/worker/service.rs b/bin/dipper-service/src/worker/service.rs index 68c91448..9887faee 100644 --- a/bin/dipper-service/src/worker/service.rs +++ b/bin/dipper-service/src/worker/service.rs @@ -41,7 +41,10 @@ const DEFAULT_QUEUE_POLL_PERIOD: Duration = Duration::from_secs(1); /// (IISA HTTP, indexer RPC, chain RPC + receipt polling), so the legitimate /// worst case is their sum, on the order of a couple of minutes. This timeout /// sits comfortably above that and only fires if a dependency accepts the -/// connection but never responds, defeating the per-call timeouts. Critically, +/// connection but never responds, defeating the per-call timeouts. It also +/// bounds how long a chain submission may retry across the RPC providers +/// (see `derive_submit_deadline`): 4/5 of it must hold two walks of a +/// 3-provider ring at the default 10s timeout and 3 retries. Critically, /// for the whole `process_job` call the job's `JobGuard` holds the row's /// `Running` lock (and the pgmq transaction behind it). An unbounded hang /// would therefore both wedge the worker loop and pin that row indefinitely. @@ -50,7 +53,7 @@ const DEFAULT_QUEUE_POLL_PERIOD: Duration = Duration::from_secs(1); /// `JobGuard` reschedules the row and releases its lock. Recovery is /// idempotent (chain-as-source-of-truth), so re-running a job whose handler /// was cancelled mid-flight is safe. -pub(crate) const PROCESS_JOB_TIMEOUT: Duration = Duration::from_secs(300); +pub(crate) const PROCESS_JOB_TIMEOUT: Duration = Duration::from_secs(420); /// Base backoff for a job rescheduled after hitting [`PROCESS_JOB_TIMEOUT`]. const JOB_TIMEOUT_RETRY_BASE_DELAY: Duration = Duration::from_secs(30); diff --git a/k8s/configmap-example.yaml b/k8s/configmap-example.yaml index f701d297..2d369960 100644 --- a/k8s/configmap-example.yaml +++ b/k8s/configmap-example.yaml @@ -117,6 +117,6 @@ data: "health": { "enabled": true, "listen_addr": "0.0.0.0:8546", - "threshold": 600 + "threshold": 840 } }