refactor: remove move_value (#2661)

* chore: bump orc-rust to 0.2.42

* refactor: remove move_value

Cargo.toml

@@ -79,7 +79,7 @@ derive_builder = "0.12"
 etcd-client = "0.11"
 futures = "0.3"
 futures-util = "0.3"
-greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "365922c3581ad954b3b3851af522b0ed0096970c" }
+greptime-proto = { git = "https://github.com/GreptimeTeam/greptime-proto.git", rev = "1d7dc6f18b310355a4c16fb453d3ca7ed09f048d" }
 humantime-serde = "1.1"
 itertools = "0.10"
 lazy_static = "1.4"

src/catalog/src/kvbackend/client.rs

@@ -25,8 +25,7 @@ use common_meta::kv_backend::{KvBackend, KvBackendRef, TxnService};
 use common_meta::rpc::store::{
     BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse, BatchPutRequest,
     BatchPutResponse, CompareAndPutRequest, CompareAndPutResponse, DeleteRangeRequest,
-    DeleteRangeResponse, MoveValueRequest, MoveValueResponse, PutRequest, PutResponse,
-    RangeRequest, RangeResponse,
+    DeleteRangeResponse, PutRequest, PutResponse, RangeRequest, RangeResponse,
 };
 use common_meta::rpc::KeyValue;
 use common_telemetry::{debug, timer};
@@ -152,20 +151,6 @@ impl KvBackend for CachedMetaKvBackend {
         }
     }
 
-    async fn move_value(&self, req: MoveValueRequest) -> Result<MoveValueResponse> {
-        let from_key = &req.from_key.clone();
-        let to_key = &req.to_key.clone();
-
-        let ret = self.kv_backend.move_value(req).await;
-
-        if ret.is_ok() {
-            self.invalidate_key(from_key).await;
-            self.invalidate_key(to_key).await;
-        }
-
-        ret
-    }
-
     async fn get(&self, key: &[u8]) -> Result<Option<KeyValue>> {
         let _timer = timer!(METRIC_CATALOG_KV_GET);
 
@@ -319,14 +304,6 @@ impl KvBackend for MetaKvBackend {
             .context(ExternalSnafu)
     }
 
-    async fn move_value(&self, req: MoveValueRequest) -> Result<MoveValueResponse> {
-        self.client
-            .move_value(req)
-            .await
-            .map_err(BoxedError::new)
-            .context(ExternalSnafu)
-    }
-
     fn as_any(&self) -> &dyn Any {
         self
     }

src/common/datasource/Cargo.toml

@@ -25,7 +25,7 @@ derive_builder.workspace = true
 futures.workspace = true
 lazy_static.workspace = true
 object-store = { workspace = true }
-orc-rust = { git = "https://github.com/WenyXu/orc-rs.git", rev = "5f6399f759ba30cb46610d06027b507949a117ca" }
+orc-rust = "0.2"
 paste = "1.0"
 regex = "1.7"
 serde.workspace = true

src/common/meta/src/kv_backend.rs

@@ -27,8 +27,7 @@ use crate::error::Error;
 use crate::rpc::store::{
     BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse, BatchPutRequest,
     BatchPutResponse, CompareAndPutRequest, CompareAndPutResponse, DeleteRangeRequest,
-    DeleteRangeResponse, MoveValueRequest, MoveValueResponse, PutRequest, PutResponse,
-    RangeRequest, RangeResponse,
+    DeleteRangeResponse, PutRequest, PutResponse, RangeRequest, RangeResponse,
 };
 use crate::rpc::KeyValue;
 
@@ -66,9 +65,6 @@ where
         req: BatchDeleteRequest,
     ) -> Result<BatchDeleteResponse, Self::Error>;
 
-    /// MoveValue atomically renames the key to the given updated key.
-    async fn move_value(&self, req: MoveValueRequest) -> Result<MoveValueResponse, Self::Error>;
-
     // The following methods are implemented based on the above methods,
     // and a higher-level interface is provided for to simplify usage.
 

src/common/meta/src/kv_backend/memory.rs

@@ -30,8 +30,7 @@ use crate::metrics::METRIC_META_TXN_REQUEST;
 use crate::rpc::store::{
     BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse, BatchPutRequest,
     BatchPutResponse, CompareAndPutRequest, CompareAndPutResponse, DeleteRangeRequest,
-    DeleteRangeResponse, MoveValueRequest, MoveValueResponse, PutRequest, PutResponse,
-    RangeRequest, RangeResponse,
+    DeleteRangeResponse, PutRequest, PutResponse, RangeRequest, RangeResponse,
 };
 use crate::rpc::KeyValue;
 
@@ -263,27 +262,6 @@ impl<T: ErrorExt + Send + Sync + 'static> KvBackend for MemoryKvBackend<T> {
 
         Ok(BatchDeleteResponse { prev_kvs })
     }
-
-    async fn move_value(&self, req: MoveValueRequest) -> Result<MoveValueResponse, Self::Error> {
-        let MoveValueRequest { from_key, to_key } = req;
-
-        let mut kvs = self.kvs.write().unwrap();
-
-        let kv = if let Some(v) = kvs.remove(&from_key) {
-            kvs.insert(to_key, v.clone());
-            Some(KeyValue {
-                key: from_key,
-                value: v,
-            })
-        } else {
-            kvs.get(&to_key).map(|v| KeyValue {
-                key: to_key,
-                value: v.clone(),
-            })
-        };
-
-        Ok(MoveValueResponse(kv))
-    }
 }
 
 #[async_trait]
@@ -358,7 +336,6 @@ mod tests {
         prepare_kv, test_kv_batch_delete, test_kv_batch_get, test_kv_compare_and_put,
         test_kv_delete_range, test_kv_put, test_kv_range, test_kv_range_2,
     };
-    use crate::kv_backend::KvBackend;
 
     async fn mock_mem_store_with_data() -> MemoryKvBackend<Error> {
         let kv_store = MemoryKvBackend::<Error>::new();
@@ -409,30 +386,6 @@ mod tests {
         test_kv_delete_range(kv_store).await;
     }
 
-    #[tokio::test]
-    async fn test_move_value() {
-        let kv_store = mock_mem_store_with_data().await;
-
-        let req = MoveValueRequest {
-            from_key: b"key1".to_vec(),
-            to_key: b"key111".to_vec(),
-        };
-
-        let resp = kv_store.move_value(req).await.unwrap();
-        assert_eq!(b"key1", resp.0.as_ref().unwrap().key());
-        assert_eq!(b"val1", resp.0.as_ref().unwrap().value());
-
-        let kv_store = mock_mem_store_with_data().await;
-
-        let req = MoveValueRequest {
-            from_key: b"notexistkey".to_vec(),
-            to_key: b"key222".to_vec(),
-        };
-
-        let resp = kv_store.move_value(req).await.unwrap();
-        assert!(resp.0.is_none());
-    }
-
     #[tokio::test]
     async fn test_batch_delete() {
         let kv_store = mock_mem_store_with_data().await;

src/common/meta/src/rpc/store.rs

@@ -21,8 +21,7 @@ use api::v1::meta::{
     BatchPutRequest as PbBatchPutRequest, BatchPutResponse as PbBatchPutResponse,
     CompareAndPutRequest as PbCompareAndPutRequest,
     CompareAndPutResponse as PbCompareAndPutResponse, DeleteRangeRequest as PbDeleteRangeRequest,
-    DeleteRangeResponse as PbDeleteRangeResponse, MoveValueRequest as PbMoveValueRequest,
-    MoveValueResponse as PbMoveValueResponse, PutRequest as PbPutRequest,
+    DeleteRangeResponse as PbDeleteRangeResponse, PutRequest as PbPutRequest,
     PutResponse as PbPutResponse, RangeRequest as PbRangeRequest, RangeResponse as PbRangeResponse,
     ResponseHeader as PbResponseHeader,
 };
@@ -794,82 +793,15 @@ impl DeleteRangeResponse {
     }
 }
 
-#[derive(Debug, Clone, Default)]
-pub struct MoveValueRequest {
-    /// If from_key dose not exist, return the value of to_key (if it exists).
-    /// If from_key exists, move the value of from_key to to_key (i.e. rename),
-    /// and return the value.
-    pub from_key: Vec<u8>,
-    pub to_key: Vec<u8>,
-}
-
-impl From<MoveValueRequest> for PbMoveValueRequest {
-    fn from(req: MoveValueRequest) -> Self {
-        Self {
-            header: None,
-            from_key: req.from_key,
-            to_key: req.to_key,
-        }
-    }
-}
-
-impl From<PbMoveValueRequest> for MoveValueRequest {
-    fn from(value: PbMoveValueRequest) -> Self {
-        Self {
-            from_key: value.from_key,
-            to_key: value.to_key,
-        }
-    }
-}
-
-impl MoveValueRequest {
-    #[inline]
-    pub fn new(from_key: impl Into<Vec<u8>>, to_key: impl Into<Vec<u8>>) -> Self {
-        Self {
-            from_key: from_key.into(),
-            to_key: to_key.into(),
-        }
-    }
-}
-
-#[derive(Debug, Clone)]
-pub struct MoveValueResponse(pub Option<KeyValue>);
-
-impl TryFrom<PbMoveValueResponse> for MoveValueResponse {
-    type Error = error::Error;
-
-    fn try_from(pb: PbMoveValueResponse) -> Result<Self> {
-        util::check_response_header(pb.header.as_ref())?;
-
-        Ok(Self(pb.kv.map(KeyValue::new)))
-    }
-}
-
-impl MoveValueResponse {
-    pub fn to_proto_resp(self, header: PbResponseHeader) -> PbMoveValueResponse {
-        PbMoveValueResponse {
-            header: Some(header),
-            kv: self.0.map(Into::into),
-        }
-    }
-
-    #[inline]
-    pub fn take_kv(&mut self) -> Option<KeyValue> {
-        self.0.take()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use api::v1::meta::{
         BatchPutRequest as PbBatchPutRequest, BatchPutResponse as PbBatchPutResponse,
         CompareAndPutRequest as PbCompareAndPutRequest,
         CompareAndPutResponse as PbCompareAndPutResponse,
         DeleteRangeRequest as PbDeleteRangeRequest, DeleteRangeResponse as PbDeleteRangeResponse,
-        KeyValue as PbKeyValue, MoveValueRequest as PbMoveValueRequest,
-        MoveValueResponse as PbMoveValueResponse, PutRequest as PbPutRequest,
-        PutResponse as PbPutResponse, RangeRequest as PbRangeRequest,
-        RangeResponse as PbRangeResponse,
+        KeyValue as PbKeyValue, PutRequest as PbPutRequest, PutResponse as PbPutResponse,
+        RangeRequest as PbRangeRequest, RangeResponse as PbRangeResponse,
     };
 
     use super::*;
@@ -1172,34 +1104,4 @@ mod tests {
         assert_eq!(b"v2".to_vec(), kv1.value().to_vec());
         assert_eq!(b"v2".to_vec(), kv1.take_value());
     }
-
-    #[test]
-    fn test_move_value_request_trans() {
-        let (from_key, to_key) = (b"test_key1".to_vec(), b"test_key2".to_vec());
-
-        let req = MoveValueRequest::new(from_key.clone(), to_key.clone());
-
-        let into_req: PbMoveValueRequest = req.into();
-        assert!(into_req.header.is_none());
-        assert_eq!(from_key, into_req.from_key);
-        assert_eq!(to_key, into_req.to_key);
-    }
-
-    #[test]
-    fn test_move_value_response_trans() {
-        let pb_res = PbMoveValueResponse {
-            header: None,
-            kv: Some(PbKeyValue {
-                key: b"k1".to_vec(),
-                value: b"v1".to_vec(),
-            }),
-        };
-
-        let mut res: MoveValueResponse = pb_res.try_into().unwrap();
-        let mut kv = res.take_kv().unwrap();
-        assert_eq!(b"k1".to_vec(), kv.key().to_vec());
-        assert_eq!(b"k1".to_vec(), kv.take_key());
-        assert_eq!(b"v1".to_vec(), kv.value().to_vec());
-        assert_eq!(b"v1".to_vec(), kv.take_value());
-    }
 }

src/common/meta/src/sequence.rs

@@ -166,8 +166,7 @@ mod tests {
     use crate::rpc::store::{
         BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse,
         BatchPutRequest, BatchPutResponse, CompareAndPutResponse, DeleteRangeRequest,
-        DeleteRangeResponse, MoveValueRequest, MoveValueResponse, PutRequest, PutResponse,
-        RangeRequest, RangeResponse,
+        DeleteRangeResponse, PutRequest, PutResponse, RangeRequest, RangeResponse,
     };
 
     #[tokio::test]
@@ -247,10 +246,6 @@ mod tests {
             async fn batch_delete(&self, _: BatchDeleteRequest) -> Result<BatchDeleteResponse> {
                 unreachable!()
             }
-
-            async fn move_value(&self, _: MoveValueRequest) -> Result<MoveValueResponse> {
-                unreachable!()
-            }
         }
 
         let kv_store = Arc::new(Noop {});

src/log-store/src/raft_engine/backend.rs

@@ -25,8 +25,7 @@ use common_meta::kv_backend::{KvBackend, TxnService};
 use common_meta::rpc::store::{
     BatchDeleteRequest, BatchDeleteResponse, BatchGetRequest, BatchGetResponse, BatchPutRequest,
     BatchPutResponse, CompareAndPutRequest, CompareAndPutResponse, DeleteRangeRequest,
-    DeleteRangeResponse, MoveValueRequest, MoveValueResponse, PutRequest, PutResponse,
-    RangeRequest, RangeResponse,
+    DeleteRangeResponse, PutRequest, PutResponse, RangeRequest, RangeResponse,
 };
 use common_meta::rpc::KeyValue;
 use common_meta::util::get_next_prefix_key;
@@ -343,10 +342,6 @@ impl KvBackend for RaftEngineBackend {
         Ok(BatchDeleteResponse { prev_kvs })
     }
 
-    async fn move_value(&self, _req: MoveValueRequest) -> Result<MoveValueResponse, Self::Error> {
-        unimplemented!()
-    }
-
     async fn get(&self, key: &[u8]) -> Result<Option<KeyValue>, Self::Error> {
         engine_get(&self.engine.read().unwrap(), key)
     }