Skip to content

Proactively reap idle connections past max_lifetime#111

Open
JSap0914 wants to merge 1 commit into
importcjj:mainfrom
JSap0914:fix/proactive-max-lifetime-reap
Open

Proactively reap idle connections past max_lifetime#111
JSap0914 wants to merge 1 commit into
importcjj:mainfrom
JSap0914:fix/proactive-max-lifetime-reap

Conversation

@JSap0914

Copy link
Copy Markdown

What

Idle connections whose max_lifetime has elapsed are now reaped proactively by the pool's periodic connection cleaner, instead of only being noticed the next time something checks a connection out.

Why

Pool::builder().max_lifetime(Some(dur)).build(manager) never started the background connection_cleaner task. That task is only ever spawned from set_conn_max_lifetime(). So a pool that gets its max_lifetime configured up front (the common case) has no proactive reaping at all: expired idle connections just sit in free_conns until a caller happens to call pool.get(), at which point validate_conn() lazily drops them.

This means max_lifetime can be silently exceeded for connections that are checked in and then never checked out again, e.g. during low-traffic periods.

Fixes #98

Fix

Pool::new_inner() now spawns the connection cleaner up front when max_lifetime is configured, mirroring the existing spawn logic already used by set_conn_max_lifetime(). The periodic cleaner (clean_rate, default 1s) then reaps expired idle connections on its own schedule, regardless of whether the pool is being actively used.

Test evidence

Added test_max_lifetime_proactive_reap, which populates a pool with a short max_lifetime, returns all connections to the idle pool, and then waits past both max_lifetime and clean_rate without ever checking a connection back out.

  • Before the fix: the idle connections are never dropped (DROPPED == 0) since the cleaner is never running. cargo test --test mobc test_max_lifetime_proactive_reap fails.
  • After the fix: the periodic cleaner reaps all 5 idle, expired connections on its own (DROPPED == 5). cargo test --test mobc test_max_lifetime_proactive_reap passes.

I also updated the tail end of the existing test_max_lifetime_lazy test: it previously asserted that an idle, expired connection was not reaped without an explicit checkout, which was really just pinning down the bug this PR fixes. It now asserts that connection is reaped too.

Full suite:

cargo test
...
test result: ok. 30 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

cargo fmt --check and cargo clippy --lib --tests are clean on the changed files (there is one pre-existing, unrelated clippy warning in tests/mobc.rs at a different test that predates this change).

Previously the connection cleaner background task was only spawned
from set_conn_max_lifetime(), never from Pool::builder().build(), so
a pool configured with max_lifetime up front never started the
periodic reaper. Idle connections that outlived max_lifetime were
only ever noticed lazily, when a caller happened to check one out.

Spawn the cleaner at pool construction time whenever max_lifetime is
configured, mirroring the existing spawn logic used by
set_conn_max_lifetime(). Idle connections are now reaped on the
regular clean_rate cadence even if nothing checks them out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

max lifetime does not work

1 participant