From a4e00e0a1679b58a2ab90ff8b5927710eedec881 Mon Sep 17 00:00:00 2001 From: Kaveh Ahmadi Date: Mon, 23 Dec 2024 11:52:51 -0800 Subject: [PATCH] fix missing fetchblob and fetch tree counters from getCounter Summary: When we setup OBC API for `SaplingBackingStore` fetch duration counters, we keep OBC API under an eden config `isOBCEnabled_`, and in the else condition we record `SaplingBackingStoreStats::fetchBlob` and `SaplingBackingStoreStats::fetchTree` FB303 counters. That is wrong for two reasons: 1- the fetchBlob and fetchTree FB303 counters are general value for all repos while the OBC API are repo specific value. Therefore these two counters doesn't have overlap and we need both in some cases. 2- On the hosts that we enable OBC API, we will miss the value of `fetch_blob_us` and `fetch_tree_us` counters from thrift getCounter functions. Because OBC API values doesn't show up in the thrift call. It is better to remove the else part and collect FB303 counters regardless of the OBC API. See test plan on a devserver that `isOBCEnabled_` was True. Reviewed By: jdelliot Differential Revision: D67550926 fbshipit-source-id: 670bc2b71052a8e6d7be6512fb91af010e24f946 --- eden/fs/store/hg/SaplingBackingStore.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/eden/fs/store/hg/SaplingBackingStore.cpp b/eden/fs/store/hg/SaplingBackingStore.cpp index 94c0e2e844e2e..7f0fe5d201196 100644 --- a/eden/fs/store/hg/SaplingBackingStore.cpp +++ b/eden/fs/store/hg/SaplingBackingStore.cpp @@ -455,9 +455,8 @@ void SaplingBackingStore::setFetchBlobCounters( if (isOBCEnabled_) { getBlobPerRepoLatencies_ += watch.elapsed().count(); - } else { - stats_->addDuration(&SaplingBackingStoreStats::fetchBlob, watch.elapsed()); } + stats_->addDuration(&SaplingBackingStoreStats::fetchBlob, watch.elapsed()); if (fetchResult == ObjectFetchContext::FetchResult::Success) { stats_->increment(&SaplingBackingStoreStats::fetchBlobSuccess); @@ -727,10 +726,9 @@ void SaplingBackingStore::processTreeImportRequests( if (promise->isFulfilled()) { if (isOBCEnabled_) { getTreePerRepoLatencies_ += watch.elapsed().count(); - } else { - stats_->addDuration( - &SaplingBackingStoreStats::fetchTree, watch.elapsed()); } + stats_->addDuration( + &SaplingBackingStoreStats::fetchTree, watch.elapsed()); stats_->increment(&SaplingBackingStoreStats::fetchTreeSuccess); if (store_.dogfoodingHost()) { stats_->increment(