Skip to content

Commit dbdf51a

Browse files
Park-JiyeonnZephyrcf
authored andcommitted
storage: stop proxy health checker on shutdown
Wake the proxy health-check worker when its connection is shut down or dropped. This prevents the worker from retaining proxy resources and waiting for the full health-check interval after the connection is gone. Signed-off-by: Park.Jiyeon <jiyeonnn2@icloud.com>
1 parent 18f9c2f commit dbdf51a

1 file changed

Lines changed: 90 additions & 2 deletions

File tree

storage/src/backend/connection.rs

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::collections::HashMap;
88
use std::io::{Read, Result};
99
use std::str::FromStr;
1010
use std::sync::atomic::{AtomicBool, AtomicI16, AtomicU64, Ordering};
11-
use std::sync::Arc;
11+
use std::sync::{Arc, Condvar, Mutex};
1212
use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
1313
use std::{fmt, thread};
1414

@@ -223,6 +223,32 @@ struct Proxy {
223223
replace_scheme: AtomicI16,
224224
}
225225

226+
#[derive(Debug, Default)]
227+
struct HealthCheckerStop {
228+
stopped: Mutex<bool>,
229+
wakeup: Condvar,
230+
}
231+
232+
impl HealthCheckerStop {
233+
fn stop(&self) {
234+
*self.stopped.lock().unwrap() = true;
235+
self.wakeup.notify_all();
236+
}
237+
238+
fn wait(&self, timeout: Duration) -> bool {
239+
let stopped = self.stopped.lock().unwrap();
240+
let (stopped, _result) = self
241+
.wakeup
242+
.wait_timeout_while(stopped, timeout, |s| !*s)
243+
.unwrap();
244+
*stopped
245+
}
246+
247+
fn is_stopped(&self) -> bool {
248+
*self.stopped.lock().unwrap()
249+
}
250+
}
251+
226252
impl Proxy {
227253
fn try_use_http(&self, url: &str) -> Option<String> {
228254
if self.replace_scheme.load(Ordering::Relaxed) == SCHEME_REVERSION_CACHE_REPLACE {
@@ -270,6 +296,7 @@ pub(crate) struct Connection {
270296
pub shutdown: AtomicBool,
271297
/// Timestamp of connection's last active request, represents as duration since UNIX_EPOCH in seconds.
272298
last_active: Arc<AtomicU64>,
299+
health_checker_stop: Arc<HealthCheckerStop>,
273300
}
274301

275302
impl Connection {
@@ -309,6 +336,7 @@ impl Connection {
309336
.unwrap()
310337
.as_secs(),
311338
)),
339+
health_checker_stop: Arc::new(HealthCheckerStop::default()),
312340
});
313341

314342
// Start proxy's health checking thread.
@@ -322,13 +350,18 @@ impl Connection {
322350
if proxy.health.ping_url.is_some() {
323351
let proxy = proxy.clone();
324352
let last_active = Arc::clone(&self.last_active);
353+
let stop = Arc::clone(&self.health_checker_stop);
325354

326355
// Spawn thread to update the health status of proxy server.
327356
thread::spawn(move || {
328357
let ping_url = proxy.health.ping_url.as_ref().unwrap();
329358
let mut last_success = true;
330359

331360
loop {
361+
if stop.is_stopped() {
362+
break;
363+
}
364+
332365
let elapsed = SystemTime::now()
333366
.duration_since(UNIX_EPOCH)
334367
.unwrap()
@@ -363,7 +396,9 @@ impl Connection {
363396
});
364397
}
365398

366-
thread::sleep(proxy.health.check_interval);
399+
if stop.wait(proxy.health.check_interval) {
400+
break;
401+
}
367402
}
368403
});
369404
}
@@ -373,6 +408,7 @@ impl Connection {
373408
/// Shutdown the connection.
374409
pub fn shutdown(&self) {
375410
self.shutdown.store(true, Ordering::Release);
411+
self.health_checker_stop.stop();
376412
}
377413

378414
#[allow(clippy::too_many_arguments)]
@@ -609,6 +645,13 @@ impl Connection {
609645
}
610646
}
611647

648+
impl Drop for Connection {
649+
fn drop(&mut self) {
650+
self.shutdown.store(true, Ordering::Release);
651+
self.health_checker_stop.stop();
652+
}
653+
}
654+
612655
#[cfg(test)]
613656
mod tests {
614657
use super::*;
@@ -797,4 +840,49 @@ mod tests {
797840
err_msg
798841
);
799842
}
843+
844+
fn health_check_connection() -> Arc<Connection> {
845+
let config = ConnectionConfig {
846+
connect_timeout: 1,
847+
proxy: ProxyConfig {
848+
url: "http://127.0.0.1:1".to_string(),
849+
ping_url: "http://127.0.0.1:1/healthy".to_string(),
850+
check_interval: 3600,
851+
check_pause_elapsed: 0,
852+
..Default::default()
853+
},
854+
..Default::default()
855+
};
856+
857+
Connection::new(&config).unwrap()
858+
}
859+
860+
fn wait_for_proxy_owners(proxy: &std::sync::Weak<Proxy>, expected: usize) {
861+
let deadline = Instant::now() + Duration::from_secs(2);
862+
while proxy.strong_count() != expected && Instant::now() < deadline {
863+
thread::sleep(Duration::from_millis(10));
864+
}
865+
assert_eq!(proxy.strong_count(), expected);
866+
}
867+
868+
#[test]
869+
fn test_shutdown_stops_proxy_health_checker() {
870+
let conn = health_check_connection();
871+
let proxy = Arc::downgrade(conn.proxy.as_ref().unwrap());
872+
873+
conn.shutdown();
874+
875+
// The connection still owns one reference; the worker must release its copy.
876+
wait_for_proxy_owners(&proxy, 1);
877+
}
878+
879+
#[test]
880+
fn test_drop_stops_proxy_health_checker() {
881+
let conn = health_check_connection();
882+
let proxy = Arc::downgrade(conn.proxy.as_ref().unwrap());
883+
884+
drop(conn);
885+
886+
wait_for_proxy_owners(&proxy, 0);
887+
}
800888
}

0 commit comments

Comments
 (0)