From 2b562a432aef8516b3c619eb73b5d977d02c7ace Mon Sep 17 00:00:00 2001 From: Xiaowei Lu Date: Thu, 19 Dec 2024 11:43:31 -0800 Subject: [PATCH] eden-client refactoring: move Thrift calls from minitop into Eden instance Summary: ## Context The stack of diffs aims to encapsulate Thrift calls from EdenFS cli commands/subcommands into the eden instance. This is done to improve the overall design and maintainability of the code. The overall purpose of the stack of diffs is to improve the code structure and make it easier to maintain and modify in the future. ## This Diff Moved thrift calls from minitop subcommand to be Eden instance methods. The API of the methods accept an optional connected thrift client so we can save one async call if the caller already has a client ready. Reviewed By: jdelliot Differential Revision: D67403258 fbshipit-source-id: fca1ce1f06b3b49e42acec1a9f0907f883470157 --- eden/fs/cli_rs/edenfs-client/src/instance.rs | 27 +++++++++++++++++++ eden/fs/cli_rs/edenfs-commands/src/minitop.rs | 16 ++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/eden/fs/cli_rs/edenfs-client/src/instance.rs b/eden/fs/cli_rs/edenfs-client/src/instance.rs index d91f0cc54f065..9cd266d1dff2d 100644 --- a/eden/fs/cli_rs/edenfs-client/src/instance.rs +++ b/eden/fs/cli_rs/edenfs-client/src/instance.rs @@ -572,6 +572,33 @@ impl EdenFsInstance { .map_err(|_| EdenFsError::Other(anyhow!("failed to call clearAndCompactLocalStore"))) } + pub async fn flush_stats_now(&self, client: Option<&EdenFsClient>) -> Result<()> { + let client = match client { + Some(client) => client, + None => &self.get_connected_thrift_client(None).await?, + }; + client + .flushStatsNow() + .await + .map_err(|_| EdenFsError::Other(anyhow!("failed to call flushstatsNow"))) + } + + pub async fn get_regex_counters( + &self, + arg_regex: &str, + client: Option<&EdenFsClient>, + ) -> Result> { + let client = match client { + Some(client) => client, + None => &self.get_connected_thrift_client(None).await?, + }; + + client + .getRegexCounters(arg_regex) + .await + .map_err(|_| EdenFsError::Other(anyhow!("failed to get regex counters"))) + } + pub async fn get_connected_thrift_client( &self, timeout: Option, diff --git a/eden/fs/cli_rs/edenfs-commands/src/minitop.rs b/eden/fs/cli_rs/edenfs-commands/src/minitop.rs index 6a85017f77de8..063cefcb0c69c 100644 --- a/eden/fs/cli_rs/edenfs-commands/src/minitop.rs +++ b/eden/fs/cli_rs/edenfs-commands/src/minitop.rs @@ -262,7 +262,9 @@ struct ImportStat { async fn get_pending_import_counts(client: &EdenFsClient) -> Result> { let mut imports = BTreeMap::::new(); - let counters = client.getRegexCounters(PENDING_COUNTER_REGEX).await?; + let counters = EdenFsInstance::global() + .get_regex_counters(PENDING_COUNTER_REGEX, Some(client)) + .await?; for import_type in IMPORT_OBJECT_TYPES { let counter_prefix = format!("store.sapling.pending_import.{}", import_type); let number_requests = counters @@ -286,7 +288,9 @@ async fn get_pending_import_counts(client: &EdenFsClient) -> Result Result> { let mut imports = BTreeMap::::new(); - let counters = client.getRegexCounters(LIVE_COUNTER_REGEX).await?; + let counters = EdenFsInstance::global() + .get_regex_counters(LIVE_COUNTER_REGEX, Some(client)) + .await?; for import_type in IMPORT_OBJECT_TYPES { let single_prefix = format!("store.sapling.live_import.{}", import_type); let batched_prefix = format!("store.sapling.live_import.batched_{}", import_type); @@ -412,7 +416,9 @@ impl Cursor { #[async_trait] impl crate::Subcommand for MinitopCmd { async fn run(&self) -> Result { - let client = EdenFsInstance::global().connect(None).await?; + let client = EdenFsInstance::global() + .get_connected_thrift_client(None) + .await?; let mut tracked_processes = TrackedProcesses::new(); let mut system = System::new(); @@ -434,7 +440,9 @@ impl crate::Subcommand for MinitopCmd { if self.interactive { queue!(stdout, terminal::Clear(terminal::ClearType::All))?; } - client.flushStatsNow().await?; + EdenFsInstance::global() + .flush_stats_now(Some(&client)) + .await?; system.refresh_processes(); cursor.refresh_terminal_size()?;