From 994957d1592b2559417f58045a78b17cc8c39be5 Mon Sep 17 00:00:00 2001
From: Andrea Campi <acampi@meta.com>
Date: Fri, 22 Nov 2024 05:48:12 -0800
Subject: [PATCH] Unconditionally enable the watchdog on methods dispatch

Summary:
## This stack

I am trying to debug a case of some methods stalling the entire reactor. See https://fb.workplace.com/groups/1708850869939124/permalink/1836334797190730/ for context.

## This diff

Earlier diffs in the stack added unconditional `.watched()` calls further down the call stack; having this optional only loses useful signal.

Reviewed By: lmvasquezg

Differential Revision: D66297353

fbshipit-source-id: 54b4ce6e4a09076e9a372dd6df2d7993dabc7170
---
 .../scs/scs_methods/src/source_control_impl.rs      | 13 -------------
 eden/mononoke/scs/scs_server/src/main.rs            |  3 +--
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/eden/mononoke/scs/scs_methods/src/source_control_impl.rs b/eden/mononoke/scs/scs_methods/src/source_control_impl.rs
index 99f6bb00ebd37..73e9168fde768 100644
--- a/eden/mononoke/scs/scs_methods/src/source_control_impl.rs
+++ b/eden/mononoke/scs/scs_methods/src/source_control_impl.rs
@@ -133,7 +133,6 @@ pub struct SourceControlServiceImpl {
     pub(crate) async_requests_queue: Option<Arc<AsyncMethodRequestQueue>>,
     identity_proxy_checker: Arc<ConnectionSecurityChecker>,
     pub(crate) acl_provider: Arc<dyn AclProvider>,
-    pub(crate) enable_futures_watchdog: bool,
     pub(crate) watchdog_max_poll: u64,
 }
 
@@ -153,7 +152,6 @@ impl SourceControlServiceImpl {
         common_config: &CommonConfig,
         factory_group: Option<Arc<FactoryGroup<2>>>,
         async_requests_queue: Option<Arc<AsyncMethodRequestQueue>>,
-        enable_futures_watchdog: bool,
         watchdog_max_poll: u64,
     ) -> Result<Self, anyhow::Error> {
         scuba_builder.add_common_server_data();
@@ -174,7 +172,6 @@ impl SourceControlServiceImpl {
             factory_group,
             async_requests_queue,
             acl_provider: app.environment().acl_provider.clone(),
-            enable_futures_watchdog,
             watchdog_max_poll,
         })
     }
@@ -985,7 +982,6 @@ macro_rules! impl_thrift_methods {
             {
                 let fut = async move {
                     let svc = self.0.clone();
-                    let enable_futures_watchdog = self.0.enable_futures_watchdog;
                     let watchdog_max_poll = self.0.watchdog_max_poll;
                     let (ctx, session_uuid) = create_ctx!(svc, $method_name, req_ctxt, $( $param_name ),*).await?;
                     let handler = {
@@ -996,14 +992,10 @@ macro_rules! impl_thrift_methods {
                             let (stats, res) = async {
                                 check_memory_usage(&ctx, stringify!($method_name), start_mem_stats.as_ref())?;
                                 let f = svc.$method_name(ctx.clone(), $( $param_name ),* );
-                                if enable_futures_watchdog {
                                     f.watched(ctx.logger())
                                     .with_label(stringify!($method_name))
                                     .with_unique_id(&session_uuid)
                                     .with_max_poll(watchdog_max_poll).await
-                                } else {
-                                    f.await
-                                }
                             }
                             .timed()
                             .on_cancel_with_data(|stats| log_cancelled(&ctx, stringify!($method_name), &stats, start_mem_stats.as_ref()))
@@ -1046,7 +1038,6 @@ macro_rules! impl_thrift_stream_methods {
             {
                 let fut = async move {
                     let svc = self.0.clone();
-                    let enable_futures_watchdog = self.0.enable_futures_watchdog;
                     let (ctx, session_uuid) = create_ctx!(svc, $method_name, req_ctxt, $( $param_name ),*).await?;
                     let handler = {
                         cloned!(ctx);
@@ -1056,14 +1047,10 @@ macro_rules! impl_thrift_stream_methods {
                             let (stats, res) = async {
                                 check_memory_usage(&ctx, stringify!($method_name), start_mem_stats.as_ref())?;
                                 let f = svc.$method_name(ctx.clone(), $( $param_name ),* );
-                                if enable_futures_watchdog {
                                     f.watched(ctx.logger())
                                     .with_label(stringify!($method_name))
                                     .with_unique_id(&session_uuid)
                                     .with_max_poll(50).await
-                                } else {
-                                    f.await
-                                }
                             }
                             .timed()
                             .on_cancel_with_data(|stats| log_cancelled(&ctx, stringify!($method_name), &stats, start_mem_stats.as_ref()))
diff --git a/eden/mononoke/scs/scs_server/src/main.rs b/eden/mononoke/scs/scs_server/src/main.rs
index 813ded119bdd4..640da87424395 100644
--- a/eden/mononoke/scs/scs_server/src/main.rs
+++ b/eden/mononoke/scs/scs_server/src/main.rs
@@ -121,7 +121,7 @@ struct ScsServerArgs {
     /// Some long-running requests are processed asynchronously by default. This flag disables that behavior; requests will fail.
     #[clap(long, default_value = "false")]
     disable_async_requests: bool,
-    /// Enable the futures watchdog; this will log stack traces for futures that take longer than 0.5 seconds to complete.
+    /// Unused, will go away soon
     #[clap(long, default_value = "false")]
     enable_futures_watchdog: bool,
     /// Sets the threshold for watchdog logging of top-level SCS methods. As a rule of thumb this should the same or lower than thrift_queue_timeout.
@@ -304,7 +304,6 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
             &app.repo_configs().common,
             maybe_factory_group,
             async_requests_queue_client,
-            args.enable_futures_watchdog,
             args.watchdog_method_max_poll,
         ))?
     };