Skip to content

Commit 36fc1a6

Browse files
author
dmitrivasilyev
committed
Fix spurious "Server from an obsolete pool" warning with dynamic pools
The stats collector was snapshotting SERVER_STATS before POOLS, creating a race window where dynamic pool GC could remove a pool between the two snapshots. This caused harmless but noisy warn-level messages every stats collection cycle for auth_query dynamic pools. Two fixes: - Reorder snapshots: POOLS first, then SERVER_STATS/CLIENT_STATS. If a pool is GC'd before the POOLS snapshot, its servers are already gone from SERVER_STATS (retain closes them before GC removes the pool). - Downgrade warn! to debug! for the residual race (matches the client case which already uses debug! on line 568).
1 parent 18f1993 commit 36fc1a6

File tree

1 file changed

+11
-6
lines changed

1 file changed

+11
-6
lines changed

src/stats/pool.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
///
1313
/// The statistics are used by administrative commands like SHOW POOLS, SHOW POOLS EXTENDED,
1414
/// and SHOW STATS to provide insights into the pooler's operation and performance.
15-
use log::{debug, error, warn};
15+
use log::{debug, error};
1616

1717
use crate::{config::PoolMode, messages::DataType, pool::PoolIdentifier};
1818
use std::borrow::Cow;
@@ -259,13 +259,15 @@ impl PoolStats {
259259
///
260260
/// A HashMap mapping pool identifiers to their aggregated statistics
261261
pub fn construct_pool_lookup() -> HashMap<PoolIdentifier, PoolStats> {
262-
// Initialize maps and get client/server statistics
263262
let mut virtual_map: HashMap<PoolIdentifier, PoolStats> = HashMap::new();
264-
let client_map = super::get_client_stats();
265-
let server_map = super::get_server_stats();
266263

267-
// Initialize statistics for each virtual pool (percentiles are calculated from HDR histograms)
264+
// Snapshot POOLS first, then SERVER_STATS/CLIENT_STATS.
265+
// This ordering reduces a race with dynamic pool GC: if a pool is removed
266+
// from POOLS before our snapshot, its servers should already be disconnected
267+
// from SERVER_STATS (retain closes them before GC removes the pool).
268268
Self::initialize_pool_stats(&mut virtual_map);
269+
let client_map = super::get_client_stats();
270+
let server_map = super::get_server_stats();
269271

270272
// Update client and server state counters
271273
Self::update_client_server_states(&mut virtual_map, &client_map, &server_map);
@@ -585,7 +587,10 @@ impl PoolStats {
585587
_ => error!("unknown server state"),
586588
}
587589
}
588-
None => warn!("Server from an obsolete pool"),
590+
// Benign race: server snapshot was taken before pool snapshot,
591+
// and the dynamic pool was GC'd in between. The server is already
592+
// gone; its stats were captured via address stats in initialize_pool_stats.
593+
None => debug!("Server from an obsolete pool"),
589594
}
590595
}
591596
}

0 commit comments

Comments
 (0)