failed attempt to generalize failpoints & make logging conditional

pageserver/src/control_plane_client.rs

@@ -15,8 +15,7 @@ use url::Url;
 use utils::{backoff, generation::Generation, id::NodeId};
 
 use crate::{
-    config::{NodeMetadata, PageServerConf},
-    virtual_file::on_fatal_io_error,
+    config::{NodeMetadata, PageServerConf}, pausable_failpoint, virtual_file::on_fatal_io_error
 };
 
 /// The Pageserver's client for using the control plane API: this is a small subset
@@ -208,7 +207,7 @@ impl ControlPlaneGenerationsApi for ControlPlaneClient {
                 .collect(),
         };
 
-        crate::tenant::pausable_failpoint!("control-plane-client-validate");
+        pausable_failpoint!("control-plane-client-validate");
 
         let response: ValidateResponse = self.retry_http_forever(&re_attach_path, request).await?;
 

pageserver/src/tenant.rs

@@ -121,28 +121,53 @@ use utils::{
 
 /// Declare a failpoint that can use the `pause` failpoint action.
 /// We don't want to block the executor thread, hence, spawn_blocking + await.
+#[macro_export]
 macro_rules! pausable_failpoint {
-    ($name:literal) => {
-        if cfg!(feature = "testing") {
-            tokio::task::spawn_blocking({
-                let current = tracing::Span::current();
-                move || {
-                    let _entered = current.entered();
-                    tracing::info!("at failpoint {}", $name);
-                    fail::fail_point!($name);
-                }
-            })
-            .await
-            .expect("spawn_blocking");
+    ($name:literal) => {{ pausable_failpoint!($name if true) }};
+    ($name:literal if $cond:expr) => {{ pausable_failpoint!($name if $cond,) }};
+    ($name:literal if $cond:expr, $($field:ident = $value:expr),*) => {{
+        struct PausableFailpoint {
+            name: &'static str,
+            log: bool,
         }
-    };
-    ($name:literal, $cond:expr) => {
-        if cfg!(feature = "testing") {
-            if $cond {
-                pausable_failpoint!($name)
+        impl PausableFailpoint {
+            #[inline(always)]
+            async fn run(self) {
+                if cfg!(feature = "testing") {
+                    tokio::task::spawn_blocking({
+                        let current = tracing::Span::current();
+                        move || {
+                            let _entered = current.entered();
+                            if self.log {
+                                tracing::info!("at failpoint {}", self.name);
+                            }
+                            fail::fail_point!($name);
+                        }
+                    })
+                    .await
+                    .expect("spawn_blocking");
+                }
             }
         }
-    };
+        const FAILPOINT: PausableFailpoint = {
+            let fp = PausableFailpoint {
+                name: $name,
+                $(
+                    $field: $value,
+                )*
+                ..{
+                    PausableFailpoint {
+                        name: "always set by macro",
+                        log: true,
+                    }
+                }
+            };
+            fp
+        };
+        if $cond && fail::{
+            FAILPOINT.run().await;
+        }
+    }};
 }
 pub(crate) use pausable_failpoint;
 

pageserver/src/tenant/tasks.rs

@@ -68,8 +68,8 @@ pub(crate) async fn concurrent_background_tasks_rate_limit_permit(
         .guard();
 
     pausable_failpoint!(
-        "initial-size-calculation-permit-pause",
-        loop_kind == BackgroundLoopKind::InitialLogicalSizeCalculation
+        "initial-size-calculation-permit-pause"
+        if loop_kind == BackgroundLoopKind::InitialLogicalSizeCalculation
     );
 
     match CONCURRENT_BACKGROUND_TASKS.acquire().await {