fix: bound Redis connect retries so an unreachable host fails fast
The previous commit switched to ConnectionManager but took its config
defaults. Two of them are unusable.
Default number_of_retries is 6, which is not a bounded retry in redis
0.27.6. Measured with a throwaway binary against a refusing host
(127.0.0.1:1, ConnectionManager::new_with_config):
retries = 0 -> returns in ~0.5 ms with IoError
retries = 1 -> returns in ~1.5 s with IoError
retries = 6 -> never returns (still hung at 300 s)
So the crate default is an infinite loop, not "six tries". With it in
place a Redis host that is down at boot makes startup hang forever: no
error, no log, and a container that reports Up while serving nothing.
The code path this replaced (get_multiplexed_async_connection) returned
in about half a millisecond, so the default quietly turned a fail-fast
boot into a silent hang.
Default connection_timeout is None, which leaves each attempt at the OS
connect timeout: roughly two minutes for a host that drops packets
instead of refusing them.
Setting retries to 1 restores fail-fast boot -- an unreachable host now
exits in under two seconds with `State(Redis(Connection refused))` --
while still absorbing a single transient refusal. Anything more is
redundant here: the manager re-enters the connect path on every
subsequent command once the connection drops, so recovery never depended
on one operation retrying many times.
Measured end-to-end against a local redis container, killing the Redis
and restarting it while the app stayed up:
before (retries 6): 61.6 s to recover, and during the outage
/debug/redis hung past a 25 s curl timeout
after (retries 1): 3.0 s to recover, and /debug/redis answers
immediately with detail=io_error
Auto-recovery still works with zero restarts; this only makes it faster
and stops the health endpoint from wedging while Redis is down.
Both values are named constants with the measurements in their docs so
the numbers can be checked and the "why only one retry" question does
not have to be answered again.
Gates: fmt clean, clippy -D warnings clean, cargo test 87 passed,
2 ignored, unchanged from the previous baseline.
This commit is contained in:
parent
1353745f58
commit
4cff3a7cee
39
src/state.rs
39
src/state.rs
|
|
@ -238,6 +238,38 @@ pub trait ReliableState: Send + Sync {
|
||||||
) -> Result<(), StateError>;
|
) -> Result<(), StateError>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Bound on a single Redis connect attempt, for both the initial connection
|
||||||
|
/// and every automatic reconnect.
|
||||||
|
///
|
||||||
|
/// `ConnectionManagerConfig` leaves this `None` by default, which leaves each
|
||||||
|
/// attempt at the OS connect timeout. Against a host that drops packets
|
||||||
|
/// instead of refusing them that is roughly two minutes per attempt. A
|
||||||
|
/// dead-but-reachable peer is far more likely to time out than to refuse, so
|
||||||
|
/// the bound has to be ours.
|
||||||
|
const REDIS_CONNECT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||||
|
|
||||||
|
/// How many times a single connect/reconnect operation retries before giving
|
||||||
|
/// up. This deliberately overrides the crate default of 6.
|
||||||
|
///
|
||||||
|
/// Measured against an unreachable host with redis 0.27.6
|
||||||
|
/// (`ConnectionManager::new_with_config`):
|
||||||
|
/// - retries = 0 -> returns in ~0.5 ms with `IoError`
|
||||||
|
/// - retries = 1 -> returns in ~1.5 s with `IoError`
|
||||||
|
/// - retries = 6 -> **never returns** (still hung at 300 s)
|
||||||
|
///
|
||||||
|
/// So the crate's own default retry count is an infinite loop in this version,
|
||||||
|
/// not a bounded retry. Left at the default, a Redis host that is down when
|
||||||
|
/// the process boots makes startup hang forever: no failure, no log, no signal,
|
||||||
|
/// and a container that reports `Up` while serving nothing. The pre-manager
|
||||||
|
/// code path (`get_multiplexed_async_connection`) returned in about half a
|
||||||
|
/// millisecond, so the default silently regressed fail-fast boot.
|
||||||
|
///
|
||||||
|
/// One retry is still useful: it absorbs a transient refusal. Anything beyond
|
||||||
|
/// that is redundant here, because the manager re-enters this path on every
|
||||||
|
/// subsequent command once the connection drops, so recovery does not depend
|
||||||
|
/// on a single operation retrying many times.
|
||||||
|
const REDIS_CONNECT_RETRIES: usize = 1;
|
||||||
|
|
||||||
pub struct RedisState {
|
pub struct RedisState {
|
||||||
connection: redis::aio::ConnectionManager,
|
connection: redis::aio::ConnectionManager,
|
||||||
encryption_key: Option<[u8; 32]>,
|
encryption_key: Option<[u8; 32]>,
|
||||||
|
|
@ -254,8 +286,13 @@ impl RedisState {
|
||||||
encryption_key: Option<[u8; 32]>,
|
encryption_key: Option<[u8; 32]>,
|
||||||
) -> Result<Self, StateError> {
|
) -> Result<Self, StateError> {
|
||||||
let client = redis::Client::open(redis_url)?;
|
let client = redis::Client::open(redis_url)?;
|
||||||
|
// Both knobs are overridden deliberately; see the constant docs.
|
||||||
|
let manager_config = redis::aio::ConnectionManagerConfig::new()
|
||||||
|
.set_number_of_retries(REDIS_CONNECT_RETRIES)
|
||||||
|
.set_connection_timeout(REDIS_CONNECT_TIMEOUT);
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
connection: redis::aio::ConnectionManager::new(client).await?,
|
connection: redis::aio::ConnectionManager::new_with_config(client, manager_config)
|
||||||
|
.await?,
|
||||||
encryption_key,
|
encryption_key,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue