Skip to content

Commit

Permalink
fix missing fetchblob and fetch tree counters from getCounter
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kavehahmadi60 authored and facebook-github-bot committed Dec 23, 2024
1 parent 48ecf79 commit a4e00e0
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions eden/fs/store/hg/SaplingBackingStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit a4e00e0

Please sign in to comment.