From f4efafd019e7e2263cf8cd83eb126ef5af15669d Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Wed, 20 Nov 2024 16:40:12 -0500 Subject: [PATCH] Separate ClientFaultReason type --- upstairs/src/client.rs | 46 +++++++++++++++++++++----------------- upstairs/src/downstairs.rs | 27 ++++++++++++---------- upstairs/src/upstairs.rs | 2 +- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/upstairs/src/client.rs b/upstairs/src/client.rs index fe5647d92..1c3d6675e 100644 --- a/upstairs/src/client.rs +++ b/upstairs/src/client.rs @@ -854,7 +854,12 @@ impl DownstairsClient { ) { let new_state = match self.state { DsState::Active | DsState::Offline - if matches!(reason, ClientStopReason::IneligibleForReplay) => + if matches!( + reason, + ClientStopReason::Fault( + ClientFaultReason::IneligibleForReplay + ) + ) => { DsState::Faulted } @@ -1195,10 +1200,10 @@ impl DownstairsClient { pub(crate) fn fault( &mut self, up_state: &UpstairsState, - reason: ClientStopReason, + reason: ClientFaultReason, ) { self.checked_state_transition(up_state, DsState::Faulted); - self.halt_io_task(reason); + self.halt_io_task(reason.into()); } /// Finishes an in-progress live repair, setting our state to `Active` @@ -2194,36 +2199,37 @@ pub(crate) enum ClientStopReason { /// Reconcile failed and we're restarting FailedReconcile, - /// Received an error from some IO - IOError, - /// Negotiation message received out of order BadNegotiationOrder, /// Negotiation says that we are incompatible Incompatible, - /// Live-repair failed - FailedLiveRepair, + /// The upstairs has requested that we deactivate + Deactivated, - /// Too many jobs in the queue - TooManyOutstandingJobs, + /// We have explicitly faulted the client + Fault(ClientFaultReason), +} - /// Too many bytes in the queue +/// Subset of [`ClientStopReason`] for faulting a client +#[derive(Debug)] +pub(crate) enum ClientFaultReason { + IOError, + FailedLiveRepair, + TooManyOutstandingJobs, TooManyOutstandingBytes, + OfflineDeactivated, + IneligibleForReplay, - /// The upstairs has requested that we deactivate - Deactivated, - - /// The test suite has requested a fault #[cfg(test)] RequestedFault, +} - /// The upstairs has requested that we deactivate when we were offline - OfflineDeactivated, - - /// The Upstairs has dropped jobs that would be needed for replay - IneligibleForReplay, +impl From for ClientStopReason { + fn from(f: ClientFaultReason) -> ClientStopReason { + ClientStopReason::Fault(f) + } } /// Response received from the I/O task diff --git a/upstairs/src/downstairs.rs b/upstairs/src/downstairs.rs index 0186eaab6..ace077f43 100644 --- a/upstairs/src/downstairs.rs +++ b/upstairs/src/downstairs.rs @@ -8,7 +8,10 @@ use std::{ use crate::{ cdt, - client::{ClientAction, ClientStopReason, DownstairsClient, EnqueueResult}, + client::{ + ClientAction, ClientFaultReason, ClientStopReason, DownstairsClient, + EnqueueResult, + }, guest::GuestBlockRes, io_limits::{IOLimitGuard, IOLimits}, live_repair::ExtentInfo, @@ -724,7 +727,7 @@ impl Downstairs { self.fault_client( client_id, up_state, - ClientStopReason::OfflineDeactivated, + ClientFaultReason::OfflineDeactivated, ); return false; } @@ -2621,20 +2624,20 @@ impl Downstairs { self.log, "downstairs failed, too many outstanding jobs {work_count}" ); - Some(ClientStopReason::TooManyOutstandingJobs) + Some(ClientFaultReason::TooManyOutstandingJobs) } else if byte_count as u64 > crate::IO_OUTSTANDING_MAX_BYTES { warn!( self.log, "downstairs failed, too many outstanding bytes {byte_count}" ); - Some(ClientStopReason::TooManyOutstandingBytes) + Some(ClientFaultReason::TooManyOutstandingBytes) } else if !self.can_replay { // XXX can this actually happen? warn!( self.log, "downstairs became ineligible for replay while offline" ); - Some(ClientStopReason::IneligibleForReplay) + Some(ClientFaultReason::IneligibleForReplay) } else { None }; @@ -2649,7 +2652,7 @@ impl Downstairs { &mut self, client_id: ClientId, up_state: &UpstairsState, - err: ClientStopReason, + err: ClientFaultReason, ) { self.skip_all_jobs(client_id); self.clients[client_id].fault(up_state, err); @@ -2713,7 +2716,7 @@ impl Downstairs { self.fault_client( i, up_state, - ClientStopReason::FailedLiveRepair, + ClientFaultReason::FailedLiveRepair, ); } DsState::LiveRepairReady => { @@ -3385,7 +3388,7 @@ impl Downstairs { self.fault_client( client_id, up_state, - ClientStopReason::IOError, + ClientFaultReason::IOError, ); } } @@ -4331,7 +4334,7 @@ struct DownstairsBackpressureConfig { #[cfg(test)] pub(crate) mod test { - use super::{ClientStopReason, Downstairs, PendingJob}; + use super::{ClientFaultReason, Downstairs, PendingJob}; use crate::{ downstairs::{LiveRepairData, LiveRepairState, ReconcileData}, live_repair::ExtentInfo, @@ -9616,7 +9619,7 @@ pub(crate) mod test { ds.fault_client( to_repair, &UpstairsState::Active, - ClientStopReason::RequestedFault, + ClientFaultReason::RequestedFault, ); ds.clients[to_repair].checked_state_transition( &UpstairsState::Active, @@ -9785,7 +9788,7 @@ pub(crate) mod test { ds.fault_client( to_repair, &UpstairsState::Active, - ClientStopReason::RequestedFault, + ClientFaultReason::RequestedFault, ); ds.clients[to_repair].checked_state_transition( &UpstairsState::Active, @@ -9941,7 +9944,7 @@ pub(crate) mod test { ds.fault_client( to_repair, &UpstairsState::Active, - ClientStopReason::RequestedFault, + ClientFaultReason::RequestedFault, ); ds.clients[to_repair].checked_state_transition( &UpstairsState::Active, diff --git a/upstairs/src/upstairs.rs b/upstairs/src/upstairs.rs index 5e4d871b6..b087b036e 100644 --- a/upstairs/src/upstairs.rs +++ b/upstairs/src/upstairs.rs @@ -1161,7 +1161,7 @@ impl Upstairs { self.downstairs.fault_client( client_id, &self.state, - crate::client::ClientStopReason::RequestedFault, + crate::client::ClientFaultReason::RequestedFault, ); done.send_ok(()); }