address comments

Author: hzxa21

grafana/risingwave-dashboard.dashboard.py

@@ -1698,6 +1698,23 @@ def section_hummock(panels):
                 ),
             ],
         ),
+        panels.timeseries_latency(
+            "Read Duration - MayExist",
+            "",
+            [
+                *quantile(
+                    lambda quantile, legend: panels.target(
+                        f"histogram_quantile({quantile}, sum(rate({metric('state_store_may_exist_duration_bucket')}[$__rate_interval])) by (le, job, instance, table_id))",
+                        f"p{legend}" + " - {{table_id}} @ {{job}} @ {{instance}}",
+                    ),
+                    [50, 90, 99, "max"],
+                ),
+                panels.target(
+                    f"sum by(le, job, instance, table_id)(rate({metric('state_store_may_exist_duration_sum')}[$__rate_interval])) / sum by(le, job, instance, table_id) (rate({metric('state_store_may_exist_duration_count')}[$__rate_interval]))",
+                    "avg - {{table_id}} {{job}} @ {{instance}}",
+                ),
+            ],
+        ),
         panels.timeseries_ops(
             "Read Bloom Filter",
             "",

grafana/risingwave-dashboard.json

@@ -109,7 +109,7 @@ impl LocalHummockStorage {
             .await
     }
 
-    pub async fn may_exist(
+    pub async fn may_exist_inner(
         &self,
         key_range: (Bound<Vec<u8>>, Bound<Vec<u8>>),
         read_options: ReadOptions,
@@ -223,7 +223,7 @@ impl LocalStateStore for LocalHummockStorage {
         key_range: (Bound<Vec<u8>>, Bound<Vec<u8>>),
         read_options: ReadOptions,
     ) -> Self::SurelyNotHaveFuture<'_> {
-        self.core.may_exist(key_range, read_options)
+        self.may_exist_inner(key_range, read_options)
     }
 }
 

src/storage/src/hummock/store/state_store.rs

@@ -831,7 +831,7 @@ impl HummockVersionReader {
             user_key_range.1.as_ref().map(UserKey::encode),
         );
         let bloom_filter_prefix_hash = if let Some(prefix_hint) = read_options.prefix_hint {
-            Sstable::hash_for_bloom_filter(&prefix_hint)
+            Sstable::hash_for_bloom_filter(&prefix_hint, table_id.table_id)
         } else {
             // only use `table_key_range` to see whether all SSTs are filtered out
             // without looking at bloom filter because prefix_hint is not provided

src/storage/src/hummock/store/version.rs

@@ -29,6 +29,7 @@ pub struct MonitoredStorageMetrics {
     pub iter_item: HistogramVec,
     pub iter_duration: HistogramVec,
     pub iter_scan_duration: HistogramVec,
+    pub may_exist_duration: HistogramVec,
 
     pub iter_in_process_counts: GenericCounterVec<AtomicU64>,
     pub write_batch_tuple_counts: GenericCounterVec<AtomicU64>,
@@ -100,7 +101,7 @@ impl MonitoredStorageMetrics {
         let opts = histogram_opts!(
             "state_store_iter_scan_duration",
             "Histogram of iterator scan time that have been issued to state store",
-            buckets,
+            buckets.clone(),
         );
         let iter_scan_duration =
             register_histogram_vec_with_registry!(opts, &["table_id"], registry).unwrap();
@@ -113,6 +114,14 @@ impl MonitoredStorageMetrics {
         )
         .unwrap();
 
+        let opts = histogram_opts!(
+            "state_store_may_exist_duration",
+            "Histogram of may exist time that have been issued to state store",
+            buckets,
+        );
+        let may_exist_duration =
+            register_histogram_vec_with_registry!(opts, &["table_id"], registry).unwrap();
+
         // ----- write_batch -----
         let write_batch_tuple_counts = register_int_counter_vec_with_registry!(
             "state_store_write_batch_tuple_counts",
@@ -160,6 +169,7 @@ impl MonitoredStorageMetrics {
             iter_item,
             iter_duration,
             iter_scan_duration,
+            may_exist_duration,
             iter_in_process_counts,
             write_batch_tuple_counts,
             write_batch_duration,

src/storage/src/monitor/monitored_storage_metrics.rs

@@ -196,7 +196,17 @@ impl<S: LocalStateStore> LocalStateStore for MonitoredStateStore<S> {
         key_range: (Bound<Vec<u8>>, Bound<Vec<u8>>),
         read_options: ReadOptions,
     ) -> Self::SurelyNotHaveFuture<'_> {
-        self.inner.may_exist(key_range, read_options)
+        async move {
+            let table_id_label = read_options.table_id.to_string();
+            let timer = self
+                .storage_metrics
+                .write_batch_duration
+                .with_label_values(&[table_id_label.as_str()])
+                .start_timer();
+            let res = self.inner.may_exist(key_range, read_options).await;
+            timer.observe_duration();
+            res
+        }
     }
 }