Skip to content

Commit

Permalink
Unconditionally enable the watchdog on methods dispatch
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andreacampi authored and facebook-github-bot committed Nov 22, 2024
1 parent 34cbdbd commit 994957d
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 15 deletions.
13 changes: 0 additions & 13 deletions eden/mononoke/scs/scs_methods/src/source_control_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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();
Expand All @@ -174,7 +172,6 @@ impl SourceControlServiceImpl {
factory_group,
async_requests_queue,
acl_provider: app.environment().acl_provider.clone(),
enable_futures_watchdog,
watchdog_max_poll,
})
}
Expand Down Expand Up @@ -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 = {
Expand All @@ -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()))
Expand Down Expand Up @@ -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);
Expand All @@ -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()))
Expand Down
3 changes: 1 addition & 2 deletions eden/mononoke/scs/scs_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
))?
};
Expand Down

0 comments on commit 994957d

Please sign in to comment.