From 31dda136809879b8e7f91f095bc378bb41b9f304 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Fri, 18 Apr 2025 16:25:48 -0700 Subject: [PATCH 1/2] fix: Revert "fix: avoid underflow of the queued_tasks metric (#1628)" This reverts commit 3ed6d6077cf987f31d35e3ff772cfbb5f81f5b73. Issue STOR-201 --- syncserver-common/src/lib.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/syncserver-common/src/lib.rs b/syncserver-common/src/lib.rs index e07df1eb..b509f325 100644 --- a/syncserver-common/src/lib.rs +++ b/syncserver-common/src/lib.rs @@ -130,18 +130,18 @@ impl BlockingThreadpool { T: Send + 'static, E: fmt::Debug + Send + InternalError + 'static, { - self.spawned_tasks.fetch_add(1, Ordering::SeqCst); + self.spawned_tasks.fetch_add(1, Ordering::Relaxed); // Ensure the counter's always decremented (whether the task completed, // was cancelled or panicked) scopeguard::defer! { - self.spawned_tasks.fetch_sub(1, Ordering::SeqCst); + self.spawned_tasks.fetch_sub(1, Ordering::Relaxed); } let active_threads = Arc::clone(&self.active_threads); let f_with_metrics = move || { - active_threads.fetch_add(1, Ordering::SeqCst); + active_threads.fetch_add(1, Ordering::Relaxed); scopeguard::defer! { - active_threads.fetch_sub(1, Ordering::SeqCst); + active_threads.fetch_sub(1, Ordering::Relaxed); } f() }; @@ -154,11 +154,8 @@ impl BlockingThreadpool { /// Return the pool's current metrics pub fn metrics(&self) -> BlockingThreadpoolMetrics { - // active_threads is decremented on a separate thread so we need a - // strong Ordering to ensure it's in sync w/ spawned_tasks (otherwise - // it could underflow queued_tasks) - let spawned_tasks = self.spawned_tasks.load(Ordering::SeqCst); - let active_threads = self.active_threads.load(Ordering::SeqCst); + let spawned_tasks = self.spawned_tasks.load(Ordering::Relaxed); + let active_threads = self.active_threads.load(Ordering::Relaxed); BlockingThreadpoolMetrics { queued_tasks: spawned_tasks - active_threads, active_threads, From 10daab06cf35cf5696aa6ed6b790d8115bfeb432 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Fri, 18 Apr 2025 16:29:15 -0700 Subject: [PATCH 2/2] fix: avoid underflow of the queued_tasks metric Closes STOR-201 --- syncserver-common/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/syncserver-common/src/lib.rs b/syncserver-common/src/lib.rs index b509f325..54830063 100644 --- a/syncserver-common/src/lib.rs +++ b/syncserver-common/src/lib.rs @@ -155,7 +155,13 @@ impl BlockingThreadpool { /// Return the pool's current metrics pub fn metrics(&self) -> BlockingThreadpoolMetrics { let spawned_tasks = self.spawned_tasks.load(Ordering::Relaxed); - let active_threads = self.active_threads.load(Ordering::Relaxed); + // active_threads is decremented on a separate thread so there's no + // Drop order guarantee of spawned_tasks decrementing before it does: + // catch the case where active_threads is larger + let active_threads = self + .active_threads + .load(Ordering::Relaxed) + .min(spawned_tasks); BlockingThreadpoolMetrics { queued_tasks: spawned_tasks - active_threads, active_threads,