Skip to content

Commit

Permalink
Disable perf sampling on iter next (MystenLabs#11007)
Browse files Browse the repository at this point in the history
## Description 

Iter `next` is called way more than seek. Sampling on seek is ok but on
next even though we are only incrementing an atomic counter, the sample
function consumes a lot of cpu.

## Test Plan 

Existing tests
  • Loading branch information
sadhansood authored Apr 17, 2023
1 parent 6b7db1c commit b4f1380
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 42 deletions.
26 changes: 2 additions & 24 deletions crates/typed-store/src/rocks/iter.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0
use std::{marker::PhantomData, sync::Arc};
use std::marker::PhantomData;

use bincode::Options;
use rocksdb::Direction;

use crate::metrics::{DBMetrics, SamplingInterval};

use super::{be_fix_int_ser, errors::TypedStoreError, RocksDBRawIter};
use serde::{de::DeserializeOwned, Serialize};

Expand All @@ -15,26 +13,15 @@ pub struct Iter<'a, K, V> {
db_iter: RocksDBRawIter<'a>,
_phantom: PhantomData<(K, V)>,
direction: Direction,
cf: String,
db_metrics: Arc<DBMetrics>,
iter_bytes_sample_interval: SamplingInterval,
is_initialized: bool,
}

impl<'a, K: DeserializeOwned, V: DeserializeOwned> Iter<'a, K, V> {
pub(super) fn new(
db_iter: RocksDBRawIter<'a>,
cf: String,
db_metrics: &Arc<DBMetrics>,
iter_bytes_sample_interval: &SamplingInterval,
) -> Self {
pub(super) fn new(db_iter: RocksDBRawIter<'a>) -> Self {
Self {
db_iter,
_phantom: PhantomData,
direction: Direction::Forward,
cf,
db_metrics: db_metrics.clone(),
iter_bytes_sample_interval: iter_bytes_sample_interval.clone(),
is_initialized: false,
}
}
Expand Down Expand Up @@ -64,19 +51,10 @@ impl<'a, K: DeserializeOwned, V: DeserializeOwned> Iterator for Iter<'a, K, V> {
.expect("Valid iterator failed to get value");
let key = config.deserialize(raw_key).ok();
let value = bcs::from_bytes(raw_value).ok();
if self.iter_bytes_sample_interval.sample() {
let total_bytes_read = (raw_key.len() + raw_value.len()) as f64;
self.db_metrics
.op_metrics
.rocksdb_iter_bytes
.with_label_values(&[&self.cf])
.observe(total_bytes_read);
}
match self.direction {
Direction::Forward => self.db_iter.next(),
Direction::Reverse => self.db_iter.prev(),
}

key.and_then(|k| value.map(|v| (k, v)))
} else {
None
Expand Down
21 changes: 3 additions & 18 deletions crates/typed-store/src/rocks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,12 +1337,7 @@ impl<'a> DBTransaction<'a> {
let db_iter = self
.transaction
.raw_iterator_cf_opt(&db.cf(), db.opts.readopts());
Iter::new(
RocksDBRawIter::OptimisticTransaction(db_iter),
db.cf.clone(),
&db.db_metrics,
&db.iter_bytes_sample_interval,
)
Iter::new(RocksDBRawIter::OptimisticTransaction(db_iter))
}

pub fn keys<K: DeserializeOwned, V: DeserializeOwned>(
Expand Down Expand Up @@ -1640,12 +1635,7 @@ where
.read_perf_ctx_metrics
.report_metrics(&self.cf);
}
Iter::new(
db_iter,
self.cf.clone(),
&self.db_metrics,
&self.iter_bytes_sample_interval,
)
Iter::new(db_iter)
}

fn safe_iter(&'a self) -> Self::SafeIterator {
Expand Down Expand Up @@ -1712,12 +1702,7 @@ where
.read_perf_ctx_metrics
.report_metrics(&self.cf);
}
Iter::new(
db_iter,
self.cf.clone(),
&self.db_metrics,
&self.iter_bytes_sample_interval,
)
Iter::new(db_iter)
}

fn keys(&'a self) -> Self::Keys {
Expand Down

0 comments on commit b4f1380

Please sign in to comment.