Skip to content

Commit 3b3185e

Browse files
committed
Fix disconnect_with_action_in_transit_reconnects_test
1 parent ef591e2 commit 3b3185e

1 file changed

Lines changed: 25 additions & 4 deletions

File tree

nativelink-worker/src/local_worker.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,28 @@ use crate::worker_utils::make_connect_worker_request;
6161
/// consider an error to have occurred.
6262
const ACTIONS_IN_TRANSIT_TIMEOUT_S: f32 = 10.;
6363

64+
/// Increments `actions_in_transit` on creation and decrements it on drop, so
65+
/// the count stays accurate even when the owning action future is aborted by
66+
/// a disconnect. A stranded count makes the disconnect handler conclude the
67+
/// in-transit actions never drained, turning every disconnect-with-work into
68+
/// a fatal error instead of a reconnect.
69+
struct ActionsInTransitGuard {
70+
actions_in_transit: Arc<AtomicU64>,
71+
}
72+
73+
impl ActionsInTransitGuard {
74+
fn new(actions_in_transit: Arc<AtomicU64>) -> Self {
75+
actions_in_transit.fetch_add(1, Ordering::Release);
76+
Self { actions_in_transit }
77+
}
78+
}
79+
80+
impl Drop for ActionsInTransitGuard {
81+
fn drop(&mut self) {
82+
self.actions_in_transit.fetch_sub(1, Ordering::Release);
83+
}
84+
}
85+
6486
/// If we lose connection to the worker api server we will wait this many seconds
6587
/// before trying to connect.
6688
const CONNECTION_RETRY_DELAY_S: f32 = 0.5;
@@ -315,7 +337,8 @@ impl<'a, T: WorkerApiClientTrait + 'static, U: RunningActionsManager> LocalWorke
315337
extra_envs.insert(name.clone(), value.into_owned());
316338
}
317339
}
318-
let actions_in_transit = self.actions_in_transit.clone();
340+
let actions_in_transit_guard =
341+
ActionsInTransitGuard::new(self.actions_in_transit.clone());
319342
let worker_id = self.worker_id.clone();
320343
let running_actions_manager = self.running_actions_manager.clone();
321344
let mut grpc_client = self.grpc_client.clone();
@@ -328,7 +351,7 @@ impl<'a, T: WorkerApiClientTrait + 'static, U: RunningActionsManager> LocalWorke
328351
.map(move |r| {
329352
// Now that we either failed or registered our action, we can
330353
// consider the action to no longer be in transit.
331-
actions_in_transit.fetch_sub(1, Ordering::Release);
354+
drop(actions_in_transit_guard);
332355
r
333356
})
334357
.and_then(|action| {
@@ -438,8 +461,6 @@ impl<'a, T: WorkerApiClientTrait + 'static, U: RunningActionsManager> LocalWorke
438461
}
439462
};
440463

441-
self.actions_in_transit.fetch_add(1, Ordering::Release);
442-
443464
let add_future_channel = add_future_channel.clone();
444465

445466
info_span!(

0 commit comments

Comments
 (0)