Use as less engine_rocks in engine_tiflash as we can (#156)

pingcap · Sep 7, 2022 · 8034235 · 8034235
1 parent 6b0ba87
commit 8034235
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 13 deletions.
diff --git a/components/engine_rocks/src/encryption.rs b/components/engine_rocks/src/encryption.rs
@@ -12,7 +12,7 @@ use rocksdb::{
 use crate::raw::Env;
 
 // Use engine::Env directly since Env is not abstracted.
-pub fn get_env(
+pub(crate) fn get_env(
     base_env: Option<Arc<Env>>,
     key_manager: Option<Arc<DataKeyManager>>,
 ) -> std::result::Result<Arc<Env>, String> {

diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs
@@ -23,7 +23,7 @@ use crate::{
 
 #[derive(Clone, Debug)]
 pub struct RocksEngine {
-    pub db: Arc<DB>,
+    db: Arc<DB>,
     shared_block_cache: bool,
 }
 

diff --git a/components/raftstore/src/engine_store_ffi/mod.rs b/components/raftstore/src/engine_store_ffi/mod.rs
@@ -18,7 +18,7 @@ use std::{
 };
 
 use encryption::DataKeyManager;
-use engine_rocks::{encryption::get_env, RocksSstIterator, RocksSstReader};
+use engine_rocks::{get_env, RocksSstIterator, RocksSstReader};
 use engine_traits::{
     EncryptionKeyManager, EncryptionMethod, FileEncryptionInfo, Iterator, Peekable, SeekKey,
     SstReader, CF_DEFAULT, CF_LOCK, CF_WRITE,
@@ -663,7 +663,7 @@ pub struct SSTFileReader {
 
 impl SSTFileReader {
     fn ffi_get_cf_file_reader(path: &str, key_manager: Option<Arc<DataKeyManager>>) -> RawVoidPtr {
-        let env = get_env(None, key_manager).unwrap();
+        let env = get_env(key_manager, None).unwrap();
         let sst_reader_res = RocksSstReader::open_with_env(path, Some(env));
         match sst_reader_res {
             Err(ref e) => tikv_util::error!("Can not open sst file {:?}", e),

diff --git a/components/sst_importer/src/sst_importer.rs b/components/sst_importer/src/sst_importer.rs
@@ -42,7 +42,7 @@ use crate::{
 /// SstImporter manages SST files that are waiting for ingesting.
 pub struct SstImporter {
     dir: ImportDir,
-    pub key_manager: Option<Arc<DataKeyManager>>,
+    key_manager: Option<Arc<DataKeyManager>>,
     switcher: ImportModeSwitcher,
     // TODO: lift api_version as a type parameter.
     api_version: ApiVersion,

diff --git a/engine_tiflash/src/engine.rs b/engine_tiflash/src/engine.rs
@@ -13,7 +13,13 @@ use std::{
     },
 };
 
-use engine_rocks::{
+use engine_rocks::{RocksDBVector, RocksEngineIterator, RocksSnapshot};
+use engine_traits::{
+    Error, IterOptions, Iterable, KvEngine, Peekable, ReadOptions, Result, SyncMutable,
+};
+use rocksdb::{DBIterator, Writable, DB};
+
+use crate::{
     options::RocksReadOptions,
     rocks_metrics::{
         flush_engine_histogram_metrics, flush_engine_iostall_properties, flush_engine_properties,
@@ -23,12 +29,7 @@ use engine_rocks::{
         ENGINE_HIST_TYPES, ENGINE_TICKER_TYPES, TITAN_ENGINE_HIST_TYPES, TITAN_ENGINE_TICKER_TYPES,
     },
     util::get_cf_handle,
-    RocksDBVector, RocksEngineIterator, RocksSnapshot,
 };
-use engine_traits::{
-    Error, IterOptions, Iterable, KvEngine, Peekable, ReadOptions, Result, SyncMutable,
-};
-use rocksdb::{DBIterator, Writable, DB};
 
 pub struct FsStatsExt {
     pub used: u64,
@@ -44,7 +45,8 @@ pub trait FFIHub: FFIHubInner + Send + Sync {}
 
 #[derive(Clone)]
 pub struct RocksEngine {
-    // Must ensure rocks is the first field, for RocksEngine::from_ref
+    // Must ensure rocks is the first field, for RocksEngine::from_ref.
+    // We must own a engine_rocks::RocksEngine, since TiKV has not decouple from engine_rocks yet.
     pub rocks: engine_rocks::RocksEngine,
     pub engine_store_server_helper: isize,
     pub pool_capacity: usize,

diff --git a/tests/proxy/normal.rs b/tests/proxy/normal.rs
@@ -981,7 +981,7 @@ mod ingest {
         region_epoch: RegionEpoch,
         keys: Vec<String>,
     ) -> (PathBuf, SstMeta, PathBuf) {
-        let path = cluster.engines.iter().last().unwrap().1.kv.rocks.path();
+        let path = cluster.engines.iter().last().unwrap().1.kv.path();
         let (import_dir, importer) = create_tmp_importer(&cluster.cfg, path);
 
         // Prepare data