From 8d8a480dc13e05f7704afb125f6b7f1adfd83dbf Mon Sep 17 00:00:00 2001
From: dennis zhuang <killme2008@gmail.com>
Date: Tue, 25 Apr 2023 21:48:51 +0800
Subject: [PATCH] fix: object store caching bug, #1466 (#1467)

* fix: object store caching bug, #1466

* fix: forgot to add S3WithCache tests
---
 src/object-store/src/cache_policy.rs |  4 ++--
 tests-integration/src/test_util.rs   | 28 +++++++++++++++++++---------
 tests-integration/tests/main.rs      |  4 ++--
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/object-store/src/cache_policy.rs b/src/object-store/src/cache_policy.rs
index cb08a26d99..81275d7d15 100644
--- a/src/object-store/src/cache_policy.rs
+++ b/src/object-store/src/cache_policy.rs
@@ -93,7 +93,7 @@ impl<I: Accessor, C: Accessor> LayeredAccessor for LruCacheAccessor<I, C> {
         let cache_path = self.cache_path(&path, &args);
         let lru_cache = self.lru_cache.clone();
 
-        match self.cache.read(&cache_path, OpRead::default()).await {
+        match self.cache.read(&cache_path, args.clone()).await {
             Ok((rp, r)) => {
                 increment_counter!(OBJECT_STORE_LRU_CACHE_HIT);
 
@@ -116,7 +116,7 @@ impl<I: Accessor, C: Accessor> LayeredAccessor for LruCacheAccessor<I, C> {
                 writer.write(Bytes::from(buf)).await?;
                 writer.close().await?;
 
-                match self.cache.read(&cache_path, OpRead::default()).await {
+                match self.cache.read(&cache_path, args.clone()).await {
                     Ok((rp, reader)) => {
                         let r = {
                             // push new cache file name to lru
diff --git a/tests-integration/src/test_util.rs b/tests-integration/src/test_util.rs
index 7c0628c988..501a6a8844 100644
--- a/tests-integration/src/test_util.rs
+++ b/tests-integration/src/test_util.rs
@@ -60,8 +60,10 @@ fn get_port() -> usize {
         .fetch_add(1, Ordering::Relaxed)
 }
 
+#[derive(Debug, Eq, PartialEq)]
 pub enum StorageType {
     S3,
+    S3WithCache,
     File,
     Oss,
 }
@@ -72,7 +74,7 @@ impl StorageType {
 
         match self {
             StorageType::File => true, // always test file
-            StorageType::S3 => {
+            StorageType::S3 | StorageType::S3WithCache => {
                 if let Ok(b) = env::var("GT_S3_BUCKET") {
                     !b.is_empty()
                 } else {
@@ -90,6 +92,16 @@ impl StorageType {
     }
 }
 
+fn s3_test_config() -> S3Config {
+    S3Config {
+        root: uuid::Uuid::new_v4().to_string(),
+        access_key_id: env::var("GT_S3_ACCESS_KEY_ID").unwrap(),
+        secret_access_key: env::var("GT_S3_ACCESS_KEY").unwrap(),
+        bucket: env::var("GT_S3_BUCKET").unwrap(),
+        ..Default::default()
+    }
+}
+
 fn get_test_store_config(
     store_type: &StorageType,
     name: &str,
@@ -124,14 +136,12 @@ fn get_test_store_config(
                 Some(TempDirGuard::Oss(TempFolder::new(&store, "/"))),
             )
         }
-        StorageType::S3 => {
-            let s3_config = S3Config {
-                root: uuid::Uuid::new_v4().to_string(),
-                access_key_id: env::var("GT_S3_ACCESS_KEY_ID").unwrap(),
-                secret_access_key: env::var("GT_S3_ACCESS_KEY").unwrap(),
-                bucket: env::var("GT_S3_BUCKET").unwrap(),
-                ..Default::default()
-            };
+        StorageType::S3 | StorageType::S3WithCache => {
+            let mut s3_config = s3_test_config();
+
+            if *store_type == StorageType::S3WithCache {
+                s3_config.cache_path = Some("/tmp/greptimedb_cache".to_string());
+            }
 
             let mut builder = S3::default();
             builder
diff --git a/tests-integration/tests/main.rs b/tests-integration/tests/main.rs
index 94d9efbc83..2b4e17bbf3 100644
--- a/tests-integration/tests/main.rs
+++ b/tests-integration/tests/main.rs
@@ -17,5 +17,5 @@ mod grpc;
 #[macro_use]
 mod http;
 
-grpc_tests!(File, S3, Oss);
-http_tests!(File, S3, Oss);
+grpc_tests!(File, S3, S3WithCache, Oss);
+http_tests!(File, S3, S3WithCache, Oss);