Add try get for metata cache for data fetch calls

Signed-off-by: Vishwas Garg <vishwasgarg14@gmail.com>

Author: vishwasgarg18

sandbox/plugins/block-cache-foyer/src/main/rust/src/foyer/foyer_cache.rs

@@ -731,6 +731,35 @@ impl FoyerCache {
         let raw = if let Some(pos) = key.find(SEPARATOR) { &key[..pos] } else { key };
         raw.trim_start_matches('/')
     }
+
+    /// Stats-aware peek: returns `Some(bytes)` on hit and updates `hit_count` +
+    /// `hit_bytes`; returns `None` on miss WITHOUT bumping `miss_count` or
+    /// `miss_bytes`. Designed for the metadata-tier shortcut probe in
+    /// [`crate::tiered_block_cache::TieredBlockCache::get`] — a miss there means
+    /// "this key wasn't a metadata entry to begin with" (column-chunk reads
+    /// dominate normal traffic), not a real cache failure, so counting it would
+    /// pollute the metadata-tier miss stats.
+    ///
+    /// `active_in_bytes` accounting is preserved (it's a backpressure metric for
+    /// concurrent in-flight reads, not a hit/miss metric, and a shortcut probe
+    /// still reads from disk).
+    pub fn try_get<'a>(&'a self, key: &'a CacheKey)
+        -> std::pin::Pin<Box<dyn std::future::Future<Output = Option<Bytes>> + Send + 'a>>
+    {
+        Box::pin(async {
+            let range_len = key.range_len() as i64;
+            let _active_guard = ActiveBytesGuard::new(&self.stats.active_in_bytes, range_len);
+            match self.inner.get(&key.as_str().to_string()).await {
+                Ok(Some(e)) => {
+                    let size = e.value().len() as i64;
+                    self.stats.hit_count.fetch_add(1, Ordering::Relaxed);
+                    self.stats.hit_bytes.fetch_add(size, Ordering::Relaxed);
+                    Some(Bytes::copy_from_slice(e.value()))
+                }
+                _ => None, // intentionally NO miss tracking — see doc above
+            }
+        })
+    }
 }
 
 impl BlockCache for FoyerCache {

sandbox/plugins/block-cache-foyer/src/main/rust/src/tests.rs

@@ -2771,3 +2771,239 @@ fn tiered_cache_key_alignment_warmup_matches_query() {
     assert!(block_on(tiered.get(&range_cache_key("seg0/_0.parquet", 7_500_000, 9_000_000))).is_none());
 }
 
+
+
+// ── try_get tests (FoyerCache) ──────────────────────────────────────────────
+
+/// `try_get` HIT path: returns `Some(bytes)` AND bumps `hit_count` + `hit_bytes`,
+/// matching the regular `get` HIT contract.
+#[test]
+fn try_get_hit_updates_hit_stats() {
+    let (cache, _dir) = test_cache();
+
+    let key = range_cache_key("seg0/_0.parquet", 100, 132);
+    let payload = Bytes::from_static(b"32_bytes_of_metadata____________");
+    assert_eq!(payload.len(), 32);
+    cache.put(&key, payload.clone());
+
+    let hits_before     = cache.stats.hit_count.load(Ordering::Relaxed);
+    let hit_bytes_before = cache.stats.hit_bytes.load(Ordering::Relaxed);
+    let misses_before    = cache.stats.miss_count.load(Ordering::Relaxed);
+    let miss_bytes_before = cache.stats.miss_bytes.load(Ordering::Relaxed);
+
+    let result = block_on(cache.try_get(&key));
+    assert_eq!(result.as_deref(), Some(payload.as_ref()));
+
+    assert_eq!(cache.stats.hit_count.load(Ordering::Relaxed),  hits_before + 1, "hit_count must bump on hit");
+    assert_eq!(cache.stats.hit_bytes.load(Ordering::Relaxed),  hit_bytes_before + 32, "hit_bytes must bump by entry size");
+    assert_eq!(cache.stats.miss_count.load(Ordering::Relaxed), misses_before, "miss_count must NOT bump on hit");
+    assert_eq!(cache.stats.miss_bytes.load(Ordering::Relaxed), miss_bytes_before, "miss_bytes must NOT bump on hit");
+}
+
+/// `try_get` MISS path: returns `None` and leaves `miss_count` + `miss_bytes` UNCHANGED.
+/// This is the whole point of the method — a miss on the metadata-tier shortcut probe
+/// is not a meaningful cache-failure signal and must not pollute miss stats.
+#[test]
+fn try_get_miss_does_not_update_miss_stats() {
+    let (cache, _dir) = test_cache();
+
+    let absent_key = range_cache_key("seg0/_0.parquet", 0, 1024);
+
+    let hits_before     = cache.stats.hit_count.load(Ordering::Relaxed);
+    let hit_bytes_before = cache.stats.hit_bytes.load(Ordering::Relaxed);
+    let misses_before    = cache.stats.miss_count.load(Ordering::Relaxed);
+    let miss_bytes_before = cache.stats.miss_bytes.load(Ordering::Relaxed);
+
+    let result = block_on(cache.try_get(&absent_key));
+    assert!(result.is_none(), "try_get on absent key returns None");
+
+    assert_eq!(cache.stats.hit_count.load(Ordering::Relaxed),  hits_before, "hit_count unchanged on miss");
+    assert_eq!(cache.stats.hit_bytes.load(Ordering::Relaxed),  hit_bytes_before, "hit_bytes unchanged on miss");
+    assert_eq!(cache.stats.miss_count.load(Ordering::Relaxed), misses_before, "miss_count must NOT bump");
+    assert_eq!(cache.stats.miss_bytes.load(Ordering::Relaxed), miss_bytes_before, "miss_bytes must NOT bump");
+}
+
+/// Direct comparison: `get` vs `try_get` on the same absent key. `get` bumps miss
+/// stats; `try_get` leaves them alone. Makes the contract difference obvious.
+#[test]
+fn try_get_vs_get_on_miss_diverge_on_miss_stats() {
+    let (cache, _dir) = test_cache();
+
+    let key_for_get     = range_cache_key("seg0/_0.parquet", 0, 1024);
+    let key_for_try_get = range_cache_key("seg0/_0.parquet", 2048, 3072);
+
+    let misses_before = cache.stats.miss_count.load(Ordering::Relaxed);
+
+    // `get` on absent key bumps miss_count.
+    assert!(block_on(cache.get(&key_for_get)).is_none());
+    let after_get = cache.stats.miss_count.load(Ordering::Relaxed);
+    assert_eq!(after_get, misses_before + 1, "get on miss bumps miss_count");
+
+    // `try_get` on absent key leaves miss_count unchanged.
+    assert!(block_on(cache.try_get(&key_for_try_get)).is_none());
+    let after_try_get = cache.stats.miss_count.load(Ordering::Relaxed);
+    assert_eq!(after_try_get, after_get, "try_get on miss must NOT bump miss_count");
+}
+
+// ── TieredBlockCache miss-stat isolation tests ──────────────────────────────
+
+/// CORE FIX: a normal data-tier read (column chunk) hits the data cache and must
+/// NOT pollute the metadata-tier miss stats. Before the `try_get` switch, every
+/// data read bumped `metadata_cache.miss_count` because the tiered get probed
+/// metadata first via `get` (which counts misses).
+#[test]
+fn tiered_get_data_hit_does_not_bump_metadata_miss() {
+    let (tiered, _data_dir, _meta_dir) = tiered_test_cache();
+
+    let col_chunk_key = range_cache_key("seg0/_0.parquet", 0, 4096);
+    tiered.put(&col_chunk_key, Bytes::from(vec![0xAB; 4096]));
+
+    let meta_misses_before = tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed);
+    let meta_miss_bytes_before = tiered.metadata_cache().stats.miss_bytes.load(Ordering::Relaxed);
+    let data_hits_before  = tiered.data_cache().stats.hit_count.load(Ordering::Relaxed);
+
+    let result = block_on(tiered.get(&col_chunk_key));
+    assert_eq!(result.map(|b| b.len()), Some(4096));
+
+    // Data tier hit count goes up.
+    assert_eq!(
+        tiered.data_cache().stats.hit_count.load(Ordering::Relaxed),
+        data_hits_before + 1,
+        "data tier should record the hit"
+    );
+    // Metadata tier miss stats stay UNCHANGED — that's the fix.
+    assert_eq!(
+        tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed),
+        meta_misses_before,
+        "metadata tier miss_count must NOT bump on data-tier hit"
+    );
+    assert_eq!(
+        tiered.metadata_cache().stats.miss_bytes.load(Ordering::Relaxed),
+        meta_miss_bytes_before,
+        "metadata tier miss_bytes must NOT bump on data-tier hit"
+    );
+}
+
+/// When BOTH tiers miss (cold key), only the data tier records the miss. The
+/// metadata tier's miss stats remain at zero — every column-chunk-shaped key
+/// would otherwise inflate them with non-meaningful misses.
+#[test]
+fn tiered_get_full_miss_only_bumps_data_miss() {
+    let (tiered, _data_dir, _meta_dir) = tiered_test_cache();
+
+    let cold_key = range_cache_key("seg0/_0.parquet", 5_000_000, 6_000_000);
+
+    let meta_misses_before = tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed);
+    let data_misses_before = tiered.data_cache().stats.miss_count.load(Ordering::Relaxed);
+
+    let result = block_on(tiered.get(&cold_key));
+    assert!(result.is_none(), "cold key must miss both tiers");
+
+    // Data tier records the real miss.
+    assert_eq!(
+        tiered.data_cache().stats.miss_count.load(Ordering::Relaxed),
+        data_misses_before + 1,
+        "data tier should record the miss"
+    );
+    // Metadata tier miss stats stay at zero.
+    assert_eq!(
+        tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed),
+        meta_misses_before,
+        "metadata tier miss_count must NOT bump on full miss"
+    );
+}
+
+/// Sanity check: a metadata HIT (warmup-promoted entry) still bumps
+/// `metadata_cache.hit_count` and `hit_bytes`. The fix must only suppress the
+/// MISS path; the HIT path is unaffected.
+#[test]
+fn tiered_get_metadata_hit_still_bumps_metadata_hit_stats() {
+    let (tiered, _data_dir, _meta_dir) = tiered_test_cache();
+
+    let footer_key = range_cache_key("seg0/_0.parquet", 9_000_000, 10_000_000);
+    let payload = Bytes::from(vec![0xCD; 1_000_000]);
+    tiered.put_metadata(&footer_key, payload.clone());
+
+    let meta_hits_before    = tiered.metadata_cache().stats.hit_count.load(Ordering::Relaxed);
+    let meta_hit_bytes_before = tiered.metadata_cache().stats.hit_bytes.load(Ordering::Relaxed);
+    let meta_misses_before  = tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed);
+    let data_hits_before    = tiered.data_cache().stats.hit_count.load(Ordering::Relaxed);
+
+    let result = block_on(tiered.get(&footer_key));
+    assert_eq!(result.map(|b| b.len()), Some(1_000_000));
+
+    // Metadata tier hit stats bump.
+    assert_eq!(
+        tiered.metadata_cache().stats.hit_count.load(Ordering::Relaxed),
+        meta_hits_before + 1,
+        "metadata hit_count must bump on metadata HIT"
+    );
+    assert_eq!(
+        tiered.metadata_cache().stats.hit_bytes.load(Ordering::Relaxed),
+        meta_hit_bytes_before + 1_000_000,
+        "metadata hit_bytes must bump by entry size on HIT"
+    );
+    // Metadata miss + data tier counters untouched.
+    assert_eq!(
+        tiered.metadata_cache().stats.miss_count.load(Ordering::Relaxed),
+        meta_misses_before,
+        "metadata miss_count must stay unchanged"
+    );
+    assert_eq!(
+        tiered.data_cache().stats.hit_count.load(Ordering::Relaxed),
+        data_hits_before,
+        "data tier untouched when metadata hits"
+    );
+}
+
+/// Realistic mixed workload: warmup populates metadata for 1 footer, query reads
+/// 1 metadata range + 5 column-chunk ranges (4 data-tier hits + 1 cold miss).
+/// After the workload:
+///   metadata: hits=1, misses=0   (the fix — was 5 with the old `get`)
+///   data:    hits=4, misses=1
+#[test]
+fn tiered_get_mixed_workload_metadata_misses_stay_zero() {
+    let (tiered, _data_dir, _meta_dir) = tiered_test_cache();
+
+    let path = "seg0/_0.parquet";
+
+    // ── Warmup: 1 footer goes to metadata tier ───────────────────────────
+    let footer_key = range_cache_key(path, 9_000_000, 10_000_000);
+    tiered.put_metadata(&footer_key, Bytes::from(vec![0xCD; 1_000_000]));
+
+    // ── Pre-populate data tier: 4 column chunks ─────────────────────────
+    let chunk_keys: Vec<_> = (0..4)
+        .map(|i| range_cache_key(path, i * 100_000, (i + 1) * 100_000))
+        .collect();
+    for k in &chunk_keys {
+        tiered.put(k, Bytes::from(vec![0x01; 100_000]));
+    }
+
+    let cold_chunk_key = range_cache_key(path, 8_000_000, 8_100_000);
+
+    // ── Query: 1 metadata read + 4 data hits + 1 data miss ──────────────
+    assert!(block_on(tiered.get(&footer_key)).is_some(),
+        "footer should hit metadata tier");
+    for k in &chunk_keys {
+        assert!(block_on(tiered.get(k)).is_some(),
+            "chunk should hit data tier");
+    }
+    assert!(block_on(tiered.get(&cold_chunk_key)).is_none(),
+        "cold chunk should miss both tiers");
+
+    // ── Assert clean metadata-tier stats ─────────────────────────────────
+    let m = tiered.metadata_cache();
+    assert_eq!(m.stats.hit_count.load(Ordering::Relaxed), 1,
+        "metadata tier should record exactly 1 hit");
+    assert_eq!(m.stats.miss_count.load(Ordering::Relaxed), 0,
+        "metadata tier miss_count must be 0 — no pollution from data reads");
+    assert_eq!(m.stats.miss_bytes.load(Ordering::Relaxed), 0,
+        "metadata tier miss_bytes must be 0");
+
+    // ── Data-tier stats look exactly like a normal cache ────────────────
+    let d = tiered.data_cache();
+    assert_eq!(d.stats.hit_count.load(Ordering::Relaxed), 4,
+        "data tier should record 4 hits");
+    assert_eq!(d.stats.miss_count.load(Ordering::Relaxed), 1,
+        "data tier should record 1 real miss (the cold chunk)");
+}

sandbox/plugins/block-cache-foyer/src/main/rust/src/tiered_block_cache.rs

@@ -153,12 +153,19 @@ impl BlockCache for TieredBlockCache {
         -> std::pin::Pin<Box<dyn std::future::Future<Output = Option<Bytes>> + Send + 'a>>
     {
         Box::pin(async move {
-            // Metadata cache first — small SSD, fast probe, never evicts.
-            // On warm restart, Foyer recovers these from disk — instant hit.
-            if let Some(bytes) = self.metadata_cache.get(key).await {
+            // Metadata cache first — small SSD, fast probe, never evicts. On warm
+            // restart, Foyer recovers these from disk so the probe is an instant hit.
+            //
+            // Use `try_get` (not `get`) for the metadata-tier probe so a miss does
+            // NOT bump `metadata_cache.miss_count`. Column-chunk reads dominate
+            // normal traffic and would otherwise inflate metadata miss stats with
+            // entries that were never going to live in the metadata tier in the
+            // first place. Hits still update `metadata_cache.hit_count` and
+            // `hit_bytes` — those numbers stay meaningful.
+            if let Some(bytes) = self.metadata_cache.try_get(key).await {
                 return Some(bytes);
             }
-            // Fall through to data cache.
+            // Fall through to data cache (real hit/miss tracking happens here).
             self.data_cache.get(key).await
         })
     }