From b1c31b1c6adf82d40e013fc136db94d4bd0a11c2 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:04:13 -0800 Subject: [PATCH 01/41] engine_*: Add IOLimiter, IOLimiterExt traits Signed-off-by: Brian Anderson --- components/engine_rocks/src/io_limiter.rs | 64 ++++++++++++++++++++++ components/engine_rocks/src/lib.rs | 2 + components/engine_traits/src/engine.rs | 1 + components/engine_traits/src/io_limiter.rs | 33 +++++++++++ components/engine_traits/src/lib.rs | 2 + 5 files changed, 102 insertions(+) create mode 100644 components/engine_rocks/src/io_limiter.rs create mode 100644 components/engine_traits/src/io_limiter.rs diff --git a/components/engine_rocks/src/io_limiter.rs b/components/engine_rocks/src/io_limiter.rs new file mode 100644 index 00000000000..31fc7f793bb --- /dev/null +++ b/components/engine_rocks/src/io_limiter.rs @@ -0,0 +1,64 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +use crate::engine::RocksEngine; +use rocksdb::RateLimiter; +use engine_traits::{IOLimiterExt, IOLimiter}; + +const PRIORITY_HIGH: u8 = 1; +const REFILL_PERIOD: i64 = 100 * 1000; +const FARENESS: i32 = 10; +const SNAP_MAX_BYTES_PER_TIME: i64 = 4 * 1024 * 1024; + +impl IOLimiterExt for RocksEngine { + type IOLimiter = RocksIOLimiter; +} + +pub struct RocksIOLimiter { + inner: RateLimiter, +} + +impl IOLimiter for RocksIOLimiter { + /// # Arguments + /// + /// - `bytes_per_sec`: controls the total write rate of compaction and flush in bytes per second. + fn new(bytes_per_sec: u64) -> Self { + RocksIOLimiter { + inner: RateLimiter::new(bytes_per_sec as i64, REFILL_PERIOD, FARENESS), + } + } + + /// Sets the rate limit in bytes per second + fn set_bytes_per_second(&self, bytes_per_sec: i64) { + self.inner.set_bytes_per_second(bytes_per_sec) + } + + /// Requests an access token to read or write bytes. If this request can not be satisfied, the call is blocked. + fn request(&self, bytes: i64) { + self.inner.request(bytes, PRIORITY_HIGH) + } + + /// Gets the max bytes that can be granted in a single burst. + /// Note: it will be less than or equal to `SNAP_MAX_BYTES_PER_TIME`. + fn get_max_bytes_per_time(&self) -> i64 { + if self.inner.get_singleburst_bytes() > SNAP_MAX_BYTES_PER_TIME { + SNAP_MAX_BYTES_PER_TIME + } else { + self.inner.get_singleburst_bytes() + } + } + + /// Gets the total bytes that have gone through the rate limiter. + fn get_total_bytes_through(&self) -> i64 { + self.inner.get_total_bytes_through(PRIORITY_HIGH) + } + + /// Gets the rate limit in bytes per second. + fn get_bytes_per_second(&self) -> i64 { + self.inner.get_bytes_per_second() + } + + /// Gets the total number of requests that have gone through rate limiter + fn get_total_requests(&self) -> i64 { + self.inner.get_total_requests(PRIORITY_HIGH) + } +} diff --git a/components/engine_rocks/src/lib.rs b/components/engine_rocks/src/lib.rs index 8b17f1d8b07..5ab542f9a18 100644 --- a/components/engine_rocks/src/lib.rs +++ b/components/engine_rocks/src/lib.rs @@ -29,6 +29,8 @@ mod engine; pub use crate::engine::*; mod import; pub use crate::import::*; +mod io_limiter; +pub use crate::io_limiter::*; mod snapshot; pub use crate::snapshot::*; mod sst; diff --git a/components/engine_traits/src/engine.rs b/components/engine_traits/src/engine.rs index ed30c131996..e07e9933e5d 100644 --- a/components/engine_traits/src/engine.rs +++ b/components/engine_traits/src/engine.rs @@ -12,6 +12,7 @@ pub trait KvEngine: + CFHandleExt + ImportExt + SstExt + + IOLimiterExt + Send + Sync + Clone diff --git a/components/engine_traits/src/io_limiter.rs b/components/engine_traits/src/io_limiter.rs new file mode 100644 index 00000000000..c82c51fa745 --- /dev/null +++ b/components/engine_traits/src/io_limiter.rs @@ -0,0 +1,33 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +/// An I/O rate limiter +/// +/// Throttles the maximum bytes per second written or read. +pub trait IOLimiterExt { + type IOLimiter: IOLimiter; +} + +pub trait IOLimiter { + /// # Arguments + /// + /// - `bytes_per_sec`: controls the total write rate of compaction and flush in bytes per second. + fn new(bytes_per_sec: u64) -> Self; + + /// Sets the rate limit in bytes per second + fn set_bytes_per_second(&self, bytes_per_sec: i64); + + /// Requests an access token to read or write bytes. If this request can not be satisfied, the call is blocked. + fn request(&self, bytes: i64); + + /// Gets the max bytes that can be granted in a single burst. + fn get_max_bytes_per_time(&self) -> i64; + + /// Gets the total bytes that have gone through the rate limiter. + fn get_total_bytes_through(&self) -> i64; + + /// Gets the rate limit in bytes per second. + fn get_bytes_per_second(&self) -> i64; + + /// Gets the total number of requests that have gone through rate limiter + fn get_total_requests(&self) -> i64; +} diff --git a/components/engine_traits/src/lib.rs b/components/engine_traits/src/lib.rs index 1396e134e45..a5e1b8b4231 100644 --- a/components/engine_traits/src/lib.rs +++ b/components/engine_traits/src/lib.rs @@ -73,6 +73,8 @@ mod engine; pub use crate::engine::*; mod import; pub use import::*; +mod io_limiter; +pub use io_limiter::*; mod snapshot; pub use crate::snapshot::*; mod sst; From 63b6aa72316341dc40dada7a12ffeaaf53d1e203 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:14:07 -0800 Subject: [PATCH 02/41] engine_traits: Define LimitReader and LimitWriter Signed-off-by: Brian Anderson --- components/engine_traits/src/io_limiter.rs | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/components/engine_traits/src/io_limiter.rs b/components/engine_traits/src/io_limiter.rs index c82c51fa745..982ca9f4647 100644 --- a/components/engine_traits/src/io_limiter.rs +++ b/components/engine_traits/src/io_limiter.rs @@ -1,5 +1,8 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. +use std::io::{Read, Result, Write}; +use std::sync::Arc; + /// An I/O rate limiter /// /// Throttles the maximum bytes per second written or read. @@ -31,3 +34,83 @@ pub trait IOLimiter { /// Gets the total number of requests that have gone through rate limiter fn get_total_requests(&self) -> i64; } + +pub struct LimitWriter<'a, T: Write, L: IOLimiter> { + limiter: Option>, + writer: &'a mut T, +} + +impl<'a, T: Write + 'a, L: IOLimiter> LimitWriter<'a, T, L> { + pub fn new(limiter: Option>, writer: &'a mut T) -> LimitWriter<'a, T, L> { + LimitWriter { limiter, writer } + } +} + +impl<'a, T: Write + 'a, L: IOLimiter> Write for LimitWriter<'a, T, L> { + fn write(&mut self, buf: &[u8]) -> Result { + let total = buf.len(); + if let Some(ref limiter) = self.limiter { + let single = limiter.get_max_bytes_per_time() as usize; + let mut curr = 0; + let mut end; + while curr < total { + if curr + single >= total { + end = total; + } else { + end = curr + single; + } + limiter.request((end - curr) as i64); + self.writer.write_all(&buf[curr..end])?; + curr = end; + } + } else { + self.writer.write_all(buf)?; + } + Ok(total) + } + + fn flush(&mut self) -> Result<()> { + self.writer.flush()?; + Ok(()) + } +} + +/// A limited reader. +/// +/// The read limits the bytes per second read from an underlying reader. +pub struct LimitReader<'a, T: Read, L: IOLimiter> { + limiter: Option>, + reader: &'a mut T, +} + +impl<'a, T: Read + 'a, L: IOLimiter> LimitReader<'a, T, L> { + /// Create a new `LimitReader`. + pub fn new(limiter: Option>, reader: &'a mut T) -> LimitReader<'a, T, L> { + LimitReader { limiter, reader } + } +} + +impl<'a, T: Read + 'a, L: IOLimiter> Read for LimitReader<'a, T, L> { + fn read(&mut self, buf: &mut [u8]) -> Result { + if let Some(ref limiter) = self.limiter { + let total = buf.len(); + let single = limiter.get_max_bytes_per_time() as usize; + let mut count = 0; + let mut curr = 0; + let mut end; + while curr < total { + if curr + single >= total { + end = total; + } else { + end = curr + single; + } + limiter.request((end - curr) as i64); + count += self.reader.read(&mut buf[curr..end])?; + curr = end; + } + Ok(count) + } else { + self.reader.read(buf) + } + } +} From 67bb896d3cf661e9d969cfc0ab5198d274ffe3a0 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:17:11 -0800 Subject: [PATCH 03/41] engine_rocks: Add limiter tests Signed-off-by: Brian Anderson --- components/engine_rocks/src/io_limiter.rs | 63 +++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/components/engine_rocks/src/io_limiter.rs b/components/engine_rocks/src/io_limiter.rs index 31fc7f793bb..4bd9d54af21 100644 --- a/components/engine_rocks/src/io_limiter.rs +++ b/components/engine_rocks/src/io_limiter.rs @@ -62,3 +62,66 @@ impl IOLimiter for RocksIOLimiter { self.inner.get_total_requests(PRIORITY_HIGH) } } + +#[cfg(test)] +mod tests { + use std::fs::{self, File}; + use std::io::{Read, Write}; + use std::sync::Arc; + use tempfile::Builder; + use engine_traits::{LimitReader, LimitWriter}; + + use super::*; + + #[test] + fn test_io_limiter() { + let limiter = RocksIOLimiter::new(10 * 1024 * 1024); + assert!(limiter.get_max_bytes_per_time() <= SNAP_MAX_BYTES_PER_TIME); + + limiter.set_bytes_per_second(20 * 1024 * 1024); + assert_eq!(limiter.get_bytes_per_second(), 20 * 1024 * 1024); + + assert_eq!(limiter.get_total_bytes_through(), 0); + + limiter.request(1024 * 1024); + assert_eq!(limiter.get_total_bytes_through(), 1024 * 1024); + + assert_eq!(limiter.get_total_requests(), 1); + } + + #[test] + fn test_limit_writer() { + let dir = Builder::new() + .prefix("_test_limit_writer") + .tempdir() + .unwrap(); + let path = dir.path().join("test-file"); + let mut file = File::create(&path).unwrap(); + let mut limit_writer = LimitWriter::new(Some(Arc::new(RocksIOLimiter::new(1024))), &mut file); + + let mut s = String::new(); + for _ in 0..100 { + s.push_str("Hello, World!"); + } + limit_writer.write_all(s.as_bytes()).unwrap(); + limit_writer.flush().unwrap(); + + let contents = fs::read_to_string(&path).unwrap(); + assert_eq!(contents, s); + } + + #[test] + fn test_limit_reader() { + let mut buf = Vec::with_capacity(512); + let bytes_per_sec = 10 * 1024 * 1024; // 10MB/s + for c in 0..1024usize { + let mut source = std::io::repeat(b'7').take(c as _); + let mut limit_reader = + LimitReader::new(Some(Arc::new(RocksIOLimiter::new(bytes_per_sec))), &mut source); + let count = limit_reader.read_to_end(&mut buf).unwrap(); + assert_eq!(count, c); + assert_eq!(count, buf.len()); + buf.clear(); + } + } +} From fe7376348e6353b5424ac0676aa1ef87191cb718 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:20:10 -0800 Subject: [PATCH 04/41] sst_importer: Use limiter from engine_traits Signed-off-by: Brian Anderson --- components/sst_importer/src/sst_importer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/sst_importer/src/sst_importer.rs b/components/sst_importer/src/sst_importer.rs index 8efa230233c..2a7b5496dc2 100644 --- a/components/sst_importer/src/sst_importer.rs +++ b/components/sst_importer/src/sst_importer.rs @@ -11,7 +11,7 @@ use crc::crc32::{self, Hasher32}; use kvproto::import_sstpb::*; use uuid::{Builder as UuidBuilder, Uuid}; -use engine::rocks::util::io_limiter::{IOLimiter, LimitReader}; +use engine_traits::{IOLimiter, LimitReader}; use engine_traits::Iterator; use engine_traits::{IngestExternalFileOptions, KvEngine}; use engine_traits::{SeekKey, SstReader, SstWriter, SstWriterBuilder}; @@ -125,7 +125,7 @@ impl SSTImporter { // open the external storage and limit the read speed. let limiter = if speed_limit > 0 { - Some(Arc::new(IOLimiter::new(speed_limit))) + Some(Arc::new(E::IOLimiter::new(speed_limit))) } else { None }; From 1bbfbfacb3a9c902674ae615bc4e5a639bec42cf Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:21:41 -0800 Subject: [PATCH 05/41] sst_importer: Remove rocks dependency Signed-off-by: Brian Anderson --- Cargo.lock | 1 - components/sst_importer/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d162567920..887dcd22031 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2709,7 +2709,6 @@ name = "sst_importer" version = "0.1.0" dependencies = [ "crc", - "engine", "engine_traits", "external_storage", "futures", diff --git a/components/sst_importer/Cargo.toml b/components/sst_importer/Cargo.toml index f9c11b1c27d..5c9b1e2f7c7 100644 --- a/components/sst_importer/Cargo.toml +++ b/components/sst_importer/Cargo.toml @@ -18,7 +18,6 @@ slog = { version = "2.3", features = ["max_level_trace", "release_max_level_debu slog-global = { version = "0.1", git = "https://github.com/breeswish/slog-global.git", rev = "0e23a5baff302a9d7bccd85f8f31e43339c2f2c1" } kvproto = { git = "https://github.com/pingcap/kvproto.git" } engine_traits = { path = "../engine_traits" } -engine = { path = "../engine" } keys = { path = "../keys" } tikv_alloc = { path = "../tikv_alloc", default-features = false } external_storage = { path = "../external_storage" } From b22f3c448c853985437027fe198d21a3ff0d17b3 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 11:27:50 -0800 Subject: [PATCH 06/41] tikv: Use IOLimiter from engine_traits Signed-off-by: Brian Anderson --- src/raftstore/store/snap.rs | 15 ++++++++------- src/raftstore/store/snap/io.rs | 6 +++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/raftstore/store/snap.rs b/src/raftstore/store/snap.rs index b4c6c5003c9..b3384769bc3 100644 --- a/src/raftstore/store/snap.rs +++ b/src/raftstore/store/snap.rs @@ -28,7 +28,8 @@ use crate::raftstore::errors::Error as RaftStoreError; use crate::raftstore::store::keys::{enc_end_key, enc_start_key}; use crate::raftstore::store::{RaftRouter, StoreMsg}; use crate::raftstore::Result as RaftStoreResult; -use engine::rocks::util::io_limiter::{IOLimiter, LimitWriter}; +use engine_traits::{IOLimiter, LimitWriter}; +use engine_rocks::RocksIOLimiter; use tikv_util::collections::{HashMap, HashMapEntry as Entry}; use tikv_util::file::{calc_crc32, delete_file_if_exist, file_exists, get_file_size, sync_dir}; use tikv_util::time::duration_to_sec; @@ -306,7 +307,7 @@ pub struct Snap { cf_index: usize, meta_file: MetaFile, size_track: Arc, - limiter: Option>, + limiter: Option>, hold_tmp_files: bool, } @@ -318,7 +319,7 @@ impl Snap { is_sending: bool, to_build: bool, deleter: Box, - limiter: Option>, + limiter: Option>, ) -> RaftStoreResult { let dir_path = dir.into(); if !dir_path.exists() { @@ -398,7 +399,7 @@ impl Snap { key: &SnapKey, size_track: Arc, deleter: Box, - limiter: Option>, + limiter: Option>, ) -> RaftStoreResult { let mut s = Snap::new(dir, key, size_track, true, true, deleter, limiter)?; s.init_for_building()?; @@ -433,7 +434,7 @@ impl Snap { snapshot_meta: SnapshotMeta, size_track: Arc, deleter: Box, - limiter: Option>, + limiter: Option>, ) -> RaftStoreResult { let mut s = Snap::new(dir, key, size_track, false, false, deleter, limiter)?; s.set_snapshot_meta(snapshot_meta)?; @@ -1003,7 +1004,7 @@ pub struct SnapManager { // directory to store snapfile. core: Arc>, router: Option, - limiter: Option>, + limiter: Option>, max_total_size: u64, } @@ -1329,7 +1330,7 @@ impl SnapManagerBuilder { } pub fn build>(&self, path: T, router: Option) -> SnapManager { let limiter = if self.max_write_bytes_per_sec > 0 { - Some(Arc::new(IOLimiter::new(self.max_write_bytes_per_sec))) + Some(Arc::new(RocksIOLimiter::new(self.max_write_bytes_per_sec))) } else { None }; diff --git a/src/raftstore/store/snap/io.rs b/src/raftstore/store/snap/io.rs index 545543a434a..0f81d07ac32 100644 --- a/src/raftstore/store/snap/io.rs +++ b/src/raftstore/store/snap/io.rs @@ -4,7 +4,7 @@ use std::io::{self, BufReader}; use std::{fs, usize}; use engine::rocks::util::get_cf_handle; -use engine::rocks::util::io_limiter::IOLimiter; +use engine_traits::IOLimiter; use engine::rocks::{IngestExternalFileOptions, Snapshot as DbSnapshot, Writable, WriteBatch, DB}; use engine::{CfName, Iterable}; use engine_rocks::{RocksEngine, RocksSstWriter, RocksSstWriterBuilder}; @@ -57,13 +57,13 @@ pub fn build_plain_cf_file( /// Build a snapshot file for the given column family in sst format. /// If there are no key-value pairs fetched, no files will be created at `path`, /// otherwise the file will be created and synchronized. -pub fn build_sst_cf_file( +pub fn build_sst_cf_file( path: &str, snap: &DbSnapshot, cf: CfName, start_key: &[u8], end_key: &[u8], - io_limiter: Option<&IOLimiter>, + io_limiter: Option<&L>, ) -> Result { let mut sst_writer = create_sst_file_writer(snap, cf, path)?; let mut stats = BuildStatistics::default(); From 1aa78cdc4e2cbd00d34acc0c573e3b24e0e92c55 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 14:02:26 -0800 Subject: [PATCH 07/41] tikv: Use IOLimiter from engine_traits Signed-off-by: Brian Anderson --- src/raftstore/store/region_snapshot.rs | 5 +++-- src/raftstore/store/snap/io.rs | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/raftstore/store/region_snapshot.rs b/src/raftstore/store/region_snapshot.rs index 4bd1c5d0861..7165f7ceaf5 100644 --- a/src/raftstore/store/region_snapshot.rs +++ b/src/raftstore/store/region_snapshot.rs @@ -412,6 +412,7 @@ mod tests { use engine::*; use engine::{ALL_CFS, CF_DEFAULT}; use engine_rocks::RocksSstWriterBuilder; + use engine_rocks::RocksIOLimiter; use engine_traits::{SstWriter, SstWriterBuilder}; use tikv_util::config::{ReadableDuration, ReadableSize}; use tikv_util::worker; @@ -1047,7 +1048,7 @@ mod tests { // Generate a snapshot let default_sst_file_path = path.path().join("default.sst"); let write_sst_file_path = path.path().join("write.sst"); - build_sst_cf_file( + build_sst_cf_file::( &default_sst_file_path.to_str().unwrap(), &Snapshot::new(Arc::clone(&engines.kv)), CF_DEFAULT, @@ -1056,7 +1057,7 @@ mod tests { None, ) .unwrap(); - build_sst_cf_file( + build_sst_cf_file::( &write_sst_file_path.to_str().unwrap(), &Snapshot::new(Arc::clone(&engines.kv)), CF_WRITE, diff --git a/src/raftstore/store/snap/io.rs b/src/raftstore/store/snap/io.rs index 0f81d07ac32..711c593b8ae 100644 --- a/src/raftstore/store/snap/io.rs +++ b/src/raftstore/store/snap/io.rs @@ -156,6 +156,7 @@ mod tests { use crate::raftstore::store::snap::tests::*; use engine::CF_DEFAULT; use tempfile::Builder; + use engine_rocks::RocksIOLimiter; struct TestStaleDetector; impl StaleDetector for TestStaleDetector { @@ -220,7 +221,7 @@ mod tests { let snap_cf_dir = Builder::new().prefix("test-snap-cf").tempdir().unwrap(); let sst_file_path = snap_cf_dir.path().join("sst"); - let stats = build_sst_cf_file( + let stats = build_sst_cf_file::( &sst_file_path.to_str().unwrap(), &DbSnapshot::new(Arc::clone(&db)), CF_DEFAULT, From 16d02f3c944b8ce092ab8205888323e036e103d0 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 15:58:14 -0800 Subject: [PATCH 08/41] engine: remove IOLimiter, LimitReader, LimitWriter Signed-off-by: Brian Anderson --- .../engine/src/rocks/util/io_limiter.rs | 208 ------------------ components/engine/src/rocks/util/mod.rs | 1 - 2 files changed, 209 deletions(-) delete mode 100644 components/engine/src/rocks/util/io_limiter.rs diff --git a/components/engine/src/rocks/util/io_limiter.rs b/components/engine/src/rocks/util/io_limiter.rs deleted file mode 100644 index 26271d24cd4..00000000000 --- a/components/engine/src/rocks/util/io_limiter.rs +++ /dev/null @@ -1,208 +0,0 @@ -// Copyright 2017 TiKV Project Authors. Licensed under Apache-2.0. - -use std::io::{Read, Result, Write}; -use std::option::Option; -use std::sync::Arc; - -use crate::rocks::RateLimiter; - -const PRIORITY_HIGH: u8 = 1; -const REFILL_PERIOD: i64 = 100 * 1000; -const FARENESS: i32 = 10; -const SNAP_MAX_BYTES_PER_TIME: i64 = 4 * 1024 * 1024; -pub const DEFAULT_SNAP_MAX_BYTES_PER_SEC: u64 = 100 * 1024 * 1024; - -/// The I/O rate limiter for RocksDB. -/// -/// Throttles the maximum bytes per second written or read. -pub struct IOLimiter { - inner: RateLimiter, -} - -impl IOLimiter { - /// # Arguments - /// - /// - `bytes_per_sec`: controls the total write rate of compaction and flush in bytes per second. - pub fn new(bytes_per_sec: u64) -> IOLimiter { - IOLimiter { - inner: RateLimiter::new(bytes_per_sec as i64, REFILL_PERIOD, FARENESS), - } - } - - /// Sets the rate limit in bytes per second - pub fn set_bytes_per_second(&self, bytes_per_sec: i64) { - self.inner.set_bytes_per_second(bytes_per_sec) - } - - /// Requests an access token to read or write bytes. If this request can not be satisfied, the call is blocked. - pub fn request(&self, bytes: i64) { - self.inner.request(bytes, PRIORITY_HIGH) - } - - /// Gets the max bytes that can be granted in a single burst. - /// Note: it will be less than or equal to `SNAP_MAX_BYTES_PER_TIME`. - pub fn get_max_bytes_per_time(&self) -> i64 { - if self.inner.get_singleburst_bytes() > SNAP_MAX_BYTES_PER_TIME { - SNAP_MAX_BYTES_PER_TIME - } else { - self.inner.get_singleburst_bytes() - } - } - - /// Gets the total bytes that have gone through the rate limiter. - pub fn get_total_bytes_through(&self) -> i64 { - self.inner.get_total_bytes_through(PRIORITY_HIGH) - } - - /// Gets the rate limit in bytes per second. - pub fn get_bytes_per_second(&self) -> i64 { - self.inner.get_bytes_per_second() - } - - /// Gets the total number of requests that have gone through rate limiter - pub fn get_total_requests(&self) -> i64 { - self.inner.get_total_requests(PRIORITY_HIGH) - } -} - -pub struct LimitWriter<'a, T: Write> { - limiter: Option>, - writer: &'a mut T, -} - -impl<'a, T: Write + 'a> LimitWriter<'a, T> { - pub fn new(limiter: Option>, writer: &'a mut T) -> LimitWriter<'a, T> { - LimitWriter { limiter, writer } - } -} - -impl<'a, T: Write + 'a> Write for LimitWriter<'a, T> { - fn write(&mut self, buf: &[u8]) -> Result { - let total = buf.len(); - if let Some(ref limiter) = self.limiter { - let single = limiter.get_max_bytes_per_time() as usize; - let mut curr = 0; - let mut end; - while curr < total { - if curr + single >= total { - end = total; - } else { - end = curr + single; - } - limiter.request((end - curr) as i64); - self.writer.write_all(&buf[curr..end])?; - curr = end; - } - } else { - self.writer.write_all(buf)?; - } - Ok(total) - } - - fn flush(&mut self) -> Result<()> { - self.writer.flush()?; - Ok(()) - } -} - -/// A limited reader. -/// -/// The read limits the bytes per second read from an underlying reader. -pub struct LimitReader<'a, T: Read> { - limiter: Option>, - reader: &'a mut T, -} - -impl<'a, T: Read + 'a> LimitReader<'a, T> { - /// Create a new `LimitReader`. - pub fn new(limiter: Option>, reader: &'a mut T) -> LimitReader<'a, T> { - LimitReader { limiter, reader } - } -} - -impl<'a, T: Read + 'a> Read for LimitReader<'a, T> { - fn read(&mut self, buf: &mut [u8]) -> Result { - if let Some(ref limiter) = self.limiter { - let total = buf.len(); - let single = limiter.get_max_bytes_per_time() as usize; - let mut count = 0; - let mut curr = 0; - let mut end; - while curr < total { - if curr + single >= total { - end = total; - } else { - end = curr + single; - } - limiter.request((end - curr) as i64); - count += self.reader.read(&mut buf[curr..end])?; - curr = end; - } - Ok(count) - } else { - self.reader.read(buf) - } - } -} - -#[cfg(test)] -mod tests { - use std::fs::{self, File}; - use std::io::Write; - use std::sync::Arc; - use tempfile::Builder; - - use super::*; - - #[test] - fn test_io_limiter() { - let limiter = IOLimiter::new(10 * 1024 * 1024); - assert!(limiter.get_max_bytes_per_time() <= SNAP_MAX_BYTES_PER_TIME); - - limiter.set_bytes_per_second(20 * 1024 * 1024); - assert_eq!(limiter.get_bytes_per_second(), 20 * 1024 * 1024); - - assert_eq!(limiter.get_total_bytes_through(), 0); - - limiter.request(1024 * 1024); - assert_eq!(limiter.get_total_bytes_through(), 1024 * 1024); - - assert_eq!(limiter.get_total_requests(), 1); - } - - #[test] - fn test_limit_writer() { - let dir = Builder::new() - .prefix("_test_limit_writer") - .tempdir() - .unwrap(); - let path = dir.path().join("test-file"); - let mut file = File::create(&path).unwrap(); - let mut limit_writer = LimitWriter::new(Some(Arc::new(IOLimiter::new(1024))), &mut file); - - let mut s = String::new(); - for _ in 0..100 { - s.push_str("Hello, World!"); - } - limit_writer.write_all(s.as_bytes()).unwrap(); - limit_writer.flush().unwrap(); - - let contents = fs::read_to_string(&path).unwrap(); - assert_eq!(contents, s); - } - - #[test] - fn test_limit_reader() { - let mut buf = Vec::with_capacity(512); - let bytes_per_sec = 10 * 1024 * 1024; // 10MB/s - for c in 0..1024usize { - let mut source = std::io::repeat(b'7').take(c as _); - let mut limit_reader = - LimitReader::new(Some(Arc::new(IOLimiter::new(bytes_per_sec))), &mut source); - let count = limit_reader.read_to_end(&mut buf).unwrap(); - assert_eq!(count, c); - assert_eq!(count, buf.len()); - buf.clear(); - } - } -} diff --git a/components/engine/src/rocks/util/mod.rs b/components/engine/src/rocks/util/mod.rs index 74b0e0949b4..f242c193a5c 100644 --- a/components/engine/src/rocks/util/mod.rs +++ b/components/engine/src/rocks/util/mod.rs @@ -3,7 +3,6 @@ pub mod config; pub mod engine_metrics; mod event_listener; -pub mod io_limiter; pub mod metrics_flusher; pub mod security; pub mod stats; From a1aaf0c73d8ce1abdabd2b0821889299ed2da598 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 16:11:32 -0800 Subject: [PATCH 09/41] tikv: Use engine_traits for IOLimiter Signed-off-by: Brian Anderson --- src/server/config.rs | 3 ++- src/server/gc_worker.rs | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index f7c56f2a61a..cef37deb050 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -5,7 +5,6 @@ use std::i32; use super::Result; use grpcio::CompressionAlgorithms; -use engine::rocks::util::io_limiter::DEFAULT_SNAP_MAX_BYTES_PER_SEC; use tikv_util::collections::HashMap; use tikv_util::config::{self, ReadableDuration, ReadableSize}; @@ -31,6 +30,8 @@ const DEFAULT_ENDPOINT_REQUEST_MAX_HANDLE_SECS: u64 = 60; // Number of rows in each chunk for streaming coprocessor. const DEFAULT_ENDPOINT_STREAM_BATCH_ROW_LIMIT: usize = 128; +const DEFAULT_SNAP_MAX_BYTES_PER_SEC: u64 = 100 * 1024 * 1024; + /// A clone of `grpc::CompressionAlgorithms` with serde supports. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] diff --git a/src/server/gc_worker.rs b/src/server/gc_worker.rs index dac02e176b4..0f67cc3c606 100644 --- a/src/server/gc_worker.rs +++ b/src/server/gc_worker.rs @@ -9,8 +9,9 @@ use std::thread::{self, Builder as ThreadBuilder, JoinHandle}; use std::time::{Duration, Instant}; use engine::rocks::util::get_cf_handle; -use engine::rocks::util::io_limiter::IOLimiter; use engine::rocks::DB; +use engine_traits::IOLimiter; +use engine_rocks::RocksIOLimiter; use engine::util::delete_all_in_range_cf; use engine::{CF_DEFAULT, CF_LOCK, CF_WRITE}; use futures::Future; @@ -166,7 +167,7 @@ struct GCRunner { raft_store_router: Option, /// Used to limit the write flow of GC. - limiter: Option, + limiter: Option, cfg: GCConfig, @@ -181,7 +182,7 @@ impl GCRunner { cfg: GCConfig, ) -> Self { let limiter = if cfg.max_write_bytes_per_sec.0 > 0 { - Some(IOLimiter::new(cfg.max_write_bytes_per_sec.0)) + Some(RocksIOLimiter::new(cfg.max_write_bytes_per_sec.0)) } else { None }; From 2f43c5844d84943c472a4957f53d5e5ec90151c0 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 16:24:16 -0800 Subject: [PATCH 10/41] backup: use IOLimiter from engine_traits Signed-off-by: Brian Anderson --- components/backup/src/endpoint.rs | 9 +++++---- components/backup/src/writer.rs | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/components/backup/src/endpoint.rs b/components/backup/src/endpoint.rs index 8f476e29fe5..19fcac99c72 100644 --- a/components/backup/src/endpoint.rs +++ b/components/backup/src/endpoint.rs @@ -7,7 +7,8 @@ use std::sync::atomic::*; use std::sync::*; use std::time::*; -use engine::rocks::util::io_limiter::IOLimiter; +use engine_traits::IOLimiter; +use engine_rocks::RocksIOLimiter; use engine::DB; use external_storage::*; use futures::lazy; @@ -64,7 +65,7 @@ impl fmt::Debug for Task { #[derive(Clone)] struct LimitedStorage { - limiter: Option>, + limiter: Option>, storage: Arc, } @@ -77,7 +78,7 @@ impl Task { let cancel = Arc::new(AtomicBool::new(false)); let limiter = if req.get_rate_limit() != 0 { - Some(Arc::new(IOLimiter::new(req.get_rate_limit() as _))) + Some(Arc::new(RocksIOLimiter::new(req.get_rate_limit() as _))) } else { None }; @@ -838,7 +839,7 @@ pub mod tests { } // TODO: check key number for each snapshot. - let limiter = Arc::new(IOLimiter::new(10 * 1024 * 1024 /* 10 MB/s */)); + let limiter = Arc::new(RocksIOLimiter::new(10 * 1024 * 1024 /* 10 MB/s */)); for (ts, len) in backup_tss { let mut req = BackupRequest::new(); req.set_start_key(vec![]); diff --git a/components/backup/src/writer.rs b/components/backup/src/writer.rs index 078261b68d4..6cd14f81ddd 100644 --- a/components/backup/src/writer.rs +++ b/components/backup/src/writer.rs @@ -3,7 +3,8 @@ use std::sync::Arc; use std::time::Instant; -use engine::rocks::util::io_limiter::{IOLimiter, LimitReader}; +use engine_traits::LimitReader; +use engine_rocks::{RocksIOLimiter}; use engine::{CF_DEFAULT, CF_WRITE, DB}; use engine_rocks::{RocksEngine, RocksSstWriter, RocksSstWriterBuilder}; use engine_traits::{SstWriter, SstWriterBuilder}; @@ -60,7 +61,7 @@ impl Writer { name: &str, cf: &'static str, buf: &mut Vec, - limiter: Option>, + limiter: Option>, storage: &dyn ExternalStorage, ) -> Result { buf.reserve(self.writer.file_size() as _); @@ -93,12 +94,12 @@ pub struct BackupWriter { name: String, default: Writer, write: Writer, - limiter: Option>, + limiter: Option>, } impl BackupWriter { /// Create a new BackupWriter. - pub fn new(db: Arc, name: &str, limiter: Option>) -> Result { + pub fn new(db: Arc, name: &str, limiter: Option>) -> Result { let default = RocksSstWriterBuilder::new() .set_in_memory(true) .set_cf(CF_DEFAULT) From f5f7837aa9f1f82d4c7a2cb5648f43058cd6495e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 17:16:57 -0800 Subject: [PATCH 11/41] *: rustfmt Signed-off-by: Brian Anderson --- components/backup/src/endpoint.rs | 4 ++-- components/backup/src/writer.rs | 10 +++++++--- components/engine_rocks/src/io_limiter.rs | 13 ++++++++----- components/sst_importer/src/sst_importer.rs | 2 +- src/raftstore/store/region_snapshot.rs | 2 +- src/raftstore/store/snap.rs | 2 +- src/raftstore/store/snap/io.rs | 4 ++-- src/server/gc_worker.rs | 4 ++-- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/components/backup/src/endpoint.rs b/components/backup/src/endpoint.rs index 19fcac99c72..303de72982d 100644 --- a/components/backup/src/endpoint.rs +++ b/components/backup/src/endpoint.rs @@ -7,9 +7,9 @@ use std::sync::atomic::*; use std::sync::*; use std::time::*; -use engine_traits::IOLimiter; -use engine_rocks::RocksIOLimiter; use engine::DB; +use engine_rocks::RocksIOLimiter; +use engine_traits::IOLimiter; use external_storage::*; use futures::lazy; use futures::prelude::Future; diff --git a/components/backup/src/writer.rs b/components/backup/src/writer.rs index 6cd14f81ddd..2bc544baa77 100644 --- a/components/backup/src/writer.rs +++ b/components/backup/src/writer.rs @@ -3,10 +3,10 @@ use std::sync::Arc; use std::time::Instant; -use engine_traits::LimitReader; -use engine_rocks::{RocksIOLimiter}; use engine::{CF_DEFAULT, CF_WRITE, DB}; +use engine_rocks::RocksIOLimiter; use engine_rocks::{RocksEngine, RocksSstWriter, RocksSstWriterBuilder}; +use engine_traits::LimitReader; use engine_traits::{SstWriter, SstWriterBuilder}; use external_storage::ExternalStorage; use kvproto::backup::File; @@ -99,7 +99,11 @@ pub struct BackupWriter { impl BackupWriter { /// Create a new BackupWriter. - pub fn new(db: Arc, name: &str, limiter: Option>) -> Result { + pub fn new( + db: Arc, + name: &str, + limiter: Option>, + ) -> Result { let default = RocksSstWriterBuilder::new() .set_in_memory(true) .set_cf(CF_DEFAULT) diff --git a/components/engine_rocks/src/io_limiter.rs b/components/engine_rocks/src/io_limiter.rs index 4bd9d54af21..35af0f168f7 100644 --- a/components/engine_rocks/src/io_limiter.rs +++ b/components/engine_rocks/src/io_limiter.rs @@ -1,8 +1,8 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. use crate::engine::RocksEngine; +use engine_traits::{IOLimiter, IOLimiterExt}; use rocksdb::RateLimiter; -use engine_traits::{IOLimiterExt, IOLimiter}; const PRIORITY_HIGH: u8 = 1; const REFILL_PERIOD: i64 = 100 * 1000; @@ -65,11 +65,11 @@ impl IOLimiter for RocksIOLimiter { #[cfg(test)] mod tests { + use engine_traits::{LimitReader, LimitWriter}; use std::fs::{self, File}; use std::io::{Read, Write}; use std::sync::Arc; use tempfile::Builder; - use engine_traits::{LimitReader, LimitWriter}; use super::*; @@ -97,7 +97,8 @@ mod tests { .unwrap(); let path = dir.path().join("test-file"); let mut file = File::create(&path).unwrap(); - let mut limit_writer = LimitWriter::new(Some(Arc::new(RocksIOLimiter::new(1024))), &mut file); + let mut limit_writer = + LimitWriter::new(Some(Arc::new(RocksIOLimiter::new(1024))), &mut file); let mut s = String::new(); for _ in 0..100 { @@ -116,8 +117,10 @@ mod tests { let bytes_per_sec = 10 * 1024 * 1024; // 10MB/s for c in 0..1024usize { let mut source = std::io::repeat(b'7').take(c as _); - let mut limit_reader = - LimitReader::new(Some(Arc::new(RocksIOLimiter::new(bytes_per_sec))), &mut source); + let mut limit_reader = LimitReader::new( + Some(Arc::new(RocksIOLimiter::new(bytes_per_sec))), + &mut source, + ); let count = limit_reader.read_to_end(&mut buf).unwrap(); assert_eq!(count, c); assert_eq!(count, buf.len()); diff --git a/components/sst_importer/src/sst_importer.rs b/components/sst_importer/src/sst_importer.rs index 2a7b5496dc2..2b3a7861575 100644 --- a/components/sst_importer/src/sst_importer.rs +++ b/components/sst_importer/src/sst_importer.rs @@ -11,8 +11,8 @@ use crc::crc32::{self, Hasher32}; use kvproto::import_sstpb::*; use uuid::{Builder as UuidBuilder, Uuid}; -use engine_traits::{IOLimiter, LimitReader}; use engine_traits::Iterator; +use engine_traits::{IOLimiter, LimitReader}; use engine_traits::{IngestExternalFileOptions, KvEngine}; use engine_traits::{SeekKey, SstReader, SstWriter, SstWriterBuilder}; use external_storage::create_storage; diff --git a/src/raftstore/store/region_snapshot.rs b/src/raftstore/store/region_snapshot.rs index 7165f7ceaf5..98eeb64325b 100644 --- a/src/raftstore/store/region_snapshot.rs +++ b/src/raftstore/store/region_snapshot.rs @@ -411,8 +411,8 @@ mod tests { use engine::Engines; use engine::*; use engine::{ALL_CFS, CF_DEFAULT}; - use engine_rocks::RocksSstWriterBuilder; use engine_rocks::RocksIOLimiter; + use engine_rocks::RocksSstWriterBuilder; use engine_traits::{SstWriter, SstWriterBuilder}; use tikv_util::config::{ReadableDuration, ReadableSize}; use tikv_util::worker; diff --git a/src/raftstore/store/snap.rs b/src/raftstore/store/snap.rs index b3384769bc3..df9da21c22a 100644 --- a/src/raftstore/store/snap.rs +++ b/src/raftstore/store/snap.rs @@ -28,8 +28,8 @@ use crate::raftstore::errors::Error as RaftStoreError; use crate::raftstore::store::keys::{enc_end_key, enc_start_key}; use crate::raftstore::store::{RaftRouter, StoreMsg}; use crate::raftstore::Result as RaftStoreResult; -use engine_traits::{IOLimiter, LimitWriter}; use engine_rocks::RocksIOLimiter; +use engine_traits::{IOLimiter, LimitWriter}; use tikv_util::collections::{HashMap, HashMapEntry as Entry}; use tikv_util::file::{calc_crc32, delete_file_if_exist, file_exists, get_file_size, sync_dir}; use tikv_util::time::duration_to_sec; diff --git a/src/raftstore/store/snap/io.rs b/src/raftstore/store/snap/io.rs index 711c593b8ae..b6fabf09d55 100644 --- a/src/raftstore/store/snap/io.rs +++ b/src/raftstore/store/snap/io.rs @@ -4,10 +4,10 @@ use std::io::{self, BufReader}; use std::{fs, usize}; use engine::rocks::util::get_cf_handle; -use engine_traits::IOLimiter; use engine::rocks::{IngestExternalFileOptions, Snapshot as DbSnapshot, Writable, WriteBatch, DB}; use engine::{CfName, Iterable}; use engine_rocks::{RocksEngine, RocksSstWriter, RocksSstWriterBuilder}; +use engine_traits::IOLimiter; use engine_traits::{SstWriter, SstWriterBuilder}; use tikv_util::codec::bytes::{BytesEncoder, CompactBytesFromFileDecoder}; @@ -155,8 +155,8 @@ mod tests { use super::*; use crate::raftstore::store::snap::tests::*; use engine::CF_DEFAULT; - use tempfile::Builder; use engine_rocks::RocksIOLimiter; + use tempfile::Builder; struct TestStaleDetector; impl StaleDetector for TestStaleDetector { diff --git a/src/server/gc_worker.rs b/src/server/gc_worker.rs index 0f67cc3c606..094bdb62dcb 100644 --- a/src/server/gc_worker.rs +++ b/src/server/gc_worker.rs @@ -10,10 +10,10 @@ use std::time::{Duration, Instant}; use engine::rocks::util::get_cf_handle; use engine::rocks::DB; -use engine_traits::IOLimiter; -use engine_rocks::RocksIOLimiter; use engine::util::delete_all_in_range_cf; use engine::{CF_DEFAULT, CF_LOCK, CF_WRITE}; +use engine_rocks::RocksIOLimiter; +use engine_traits::IOLimiter; use futures::Future; use kvproto::kvrpcpb::Context; use kvproto::metapb; From 743ff9a4103ea4c19f6cf3ca1a2525eb209818c8 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:28:10 +0100 Subject: [PATCH 12/41] engine_*: use i64 for rate limiter bytes_per_sec Signed-off-by: Brian Anderson --- cmd/src/server.rs | 8 ++++++-- components/engine_rocks/src/io_limiter.rs | 4 ++-- components/engine_traits/src/io_limiter.rs | 2 +- src/import/sst_service.rs | 13 +++++++++++-- src/raftstore/store/snap.rs | 4 ++-- src/server/gc_worker.rs | 5 ++++- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/cmd/src/server.rs b/cmd/src/server.rs index b7274c08e27..bdee53e1c41 100644 --- a/cmd/src/server.rs +++ b/cmd/src/server.rs @@ -12,6 +12,7 @@ use kvproto::deadlock::create_deadlock; use kvproto::debugpb::create_debug; use kvproto::import_sstpb::create_import_sst; use pd_client::{PdClient, RpcClient}; +use std::convert::TryFrom; use std::fs::File; use std::path::Path; use std::sync::{Arc, Mutex}; @@ -203,11 +204,14 @@ fn run_raft_server(pd_client: RpcClient, cfg: &TiKvConfig, security_mgr: Arc i64::max_value")); // Create snapshot manager, server. let snap_mgr = SnapManagerBuilder::default() - .max_write_bytes_per_sec(cfg.server.snap_max_write_bytes_per_sec.0) + .max_write_bytes_per_sec(bps) .max_total_size(cfg.server.snap_max_total_size.0) .build( snap_path.as_path().to_str().unwrap().to_owned(), diff --git a/components/engine_rocks/src/io_limiter.rs b/components/engine_rocks/src/io_limiter.rs index 35af0f168f7..9516a471fcd 100644 --- a/components/engine_rocks/src/io_limiter.rs +++ b/components/engine_rocks/src/io_limiter.rs @@ -21,9 +21,9 @@ impl IOLimiter for RocksIOLimiter { /// # Arguments /// /// - `bytes_per_sec`: controls the total write rate of compaction and flush in bytes per second. - fn new(bytes_per_sec: u64) -> Self { + fn new(bytes_per_sec: i64) -> Self { RocksIOLimiter { - inner: RateLimiter::new(bytes_per_sec as i64, REFILL_PERIOD, FARENESS), + inner: RateLimiter::new(bytes_per_sec, REFILL_PERIOD, FARENESS), } } diff --git a/components/engine_traits/src/io_limiter.rs b/components/engine_traits/src/io_limiter.rs index 982ca9f4647..042eaf4248c 100644 --- a/components/engine_traits/src/io_limiter.rs +++ b/components/engine_traits/src/io_limiter.rs @@ -14,7 +14,7 @@ pub trait IOLimiter { /// # Arguments /// /// - `bytes_per_sec`: controls the total write rate of compaction and flush in bytes per second. - fn new(bytes_per_sec: u64) -> Self; + fn new(bytes_per_sec: i64) -> Self; /// Sets the rate limit in bytes per second fn set_bytes_per_second(&self, bytes_per_sec: i64); diff --git a/src/import/sst_service.rs b/src/import/sst_service.rs index ac1431455f7..6523ac3376d 100644 --- a/src/import/sst_service.rs +++ b/src/import/sst_service.rs @@ -1,6 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. use std::sync::{Arc, Mutex}; +use std::convert::TryFrom; use engine::rocks::util::compact_files_in_range; use engine::rocks::DB; @@ -294,9 +295,17 @@ impl ImportSst for ImportSSTService { let label = "set_download_speed_limit"; let timer = Instant::now_coarse(); - match (req.get_speed_limit(), &mut self.limiter) { + let s = i64::try_from(req.get_speed_limit()); + let s = if let Ok(s) = s { + s + } else { + warn!("SetDownloadSpeedLimitRequest out of range: {}. Using i64::max_value", req.get_speed_limit()); + i64::max_value() + }; + + match (s, &mut self.limiter) { (0, limiter) => *limiter = None, - (s, Some(l)) => l.set_bytes_per_second(s as i64), + (s, Some(l)) => l.set_bytes_per_second(s), (s, limiter) => *limiter = Some(Arc::new(RocksIOLimiter::new(s))), } diff --git a/src/raftstore/store/snap.rs b/src/raftstore/store/snap.rs index df9da21c22a..79a56d6a6f8 100644 --- a/src/raftstore/store/snap.rs +++ b/src/raftstore/store/snap.rs @@ -1315,12 +1315,12 @@ impl SnapshotDeleter for SnapManager { #[derive(Debug, Default)] pub struct SnapManagerBuilder { - max_write_bytes_per_sec: u64, + max_write_bytes_per_sec: i64, max_total_size: u64, } impl SnapManagerBuilder { - pub fn max_write_bytes_per_sec(&mut self, bytes: u64) -> &mut SnapManagerBuilder { + pub fn max_write_bytes_per_sec(&mut self, bytes: i64) -> &mut SnapManagerBuilder { self.max_write_bytes_per_sec = bytes; self } diff --git a/src/server/gc_worker.rs b/src/server/gc_worker.rs index 094bdb62dcb..b0f57686978 100644 --- a/src/server/gc_worker.rs +++ b/src/server/gc_worker.rs @@ -1,5 +1,6 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. +use std::convert::TryFrom; use std::cmp::Ordering; use std::fmt::{self, Display, Formatter}; use std::mem; @@ -182,7 +183,9 @@ impl GCRunner { cfg: GCConfig, ) -> Self { let limiter = if cfg.max_write_bytes_per_sec.0 > 0 { - Some(RocksIOLimiter::new(cfg.max_write_bytes_per_sec.0)) + let bps = i64::try_from(cfg.max_write_bytes_per_sec.0) + .expect("snap_max_write_bytes_per_sec > i64::max_value"); + Some(RocksIOLimiter::new(bps)) } else { None }; From 36c6855c800cede7de929ce70fe79a8b078a0754 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 4 Nov 2019 18:11:36 -0800 Subject: [PATCH 13/41] engine_rocks: Add extension method for creating RocksEngine references Signed-off-by: Brian Anderson --- components/engine_rocks/src/compat.rs | 21 +++++++++++++++++++++ components/engine_rocks/src/lib.rs | 3 +++ components/engine_traits/src/lib.rs | 3 +++ 3 files changed, 27 insertions(+) create mode 100644 components/engine_rocks/src/compat.rs diff --git a/components/engine_rocks/src/compat.rs b/components/engine_rocks/src/compat.rs new file mode 100644 index 00000000000..9ee9c0fbf77 --- /dev/null +++ b/components/engine_rocks/src/compat.rs @@ -0,0 +1,21 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +use crate::engine::RocksEngine; +use engine::DB; +use std::sync::Arc; + +/// A trait to enter the world of engine traits from a raw `Arc` +/// with as little syntax as possible. +/// +/// This will be used during the transition from RocksDB to the +/// `KvEngine` abstraction and then discarded. +pub trait EngineCompat { + fn c(&self) -> &RocksEngine; +} + +impl EngineCompat for Arc { + #[inline] + fn c(&self) -> &RocksEngine { + RocksEngine::from_ref(self) + } +} diff --git a/components/engine_rocks/src/lib.rs b/components/engine_rocks/src/lib.rs index 5ab542f9a18..51962fd376e 100644 --- a/components/engine_rocks/src/lib.rs +++ b/components/engine_rocks/src/lib.rs @@ -43,3 +43,6 @@ pub use crate::engine_iterator::*; mod options; pub mod util; + +mod compat; +pub use compat::*; diff --git a/components/engine_traits/src/lib.rs b/components/engine_traits/src/lib.rs index a5e1b8b4231..4cb98734876 100644 --- a/components/engine_traits/src/lib.rs +++ b/components/engine_traits/src/lib.rs @@ -49,6 +49,9 @@ //! - Port methods directly from the existing `engine` crate by re-implementing //! it in engine_traits and engine_rocks, replacing all the callers with calls //! into the traits, then delete the versions in the `engine` crate. +//! +//! - Use the .c() method from engine_rocks::compat::EngineCompat to get a +//! KvEngine reference from Arc in the fewest characters. #![recursion_limit = "200"] From 7515f1cf0707a647df92622e7c9a4ce3049fe188 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:15:40 +0000 Subject: [PATCH 14/41] engine_rocks: Rename Snapshot to RocksSnapshot for consistency Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine.rs | 12 ++++---- components/engine_rocks/src/snapshot.rs | 40 ++++++++++++------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index 16eb9b2c246..0b8e34da320 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -11,7 +11,7 @@ use rocksdb::{DBIterator, Writable, DB}; use crate::options::{RocksReadOptions, RocksWriteOptions}; use crate::util::get_cf_handle; -use crate::{RocksEngineIterator, Snapshot}; +use crate::{RocksEngineIterator, RocksSnapshot}; #[derive(Clone, Debug)] #[repr(transparent)] @@ -48,7 +48,7 @@ impl RocksEngine { } impl KvEngine for RocksEngine { - type Snapshot = Snapshot; + type Snapshot = RocksSnapshot; type WriteBatch = crate::WriteBatch; fn write_opt(&self, opts: &WriteOptions, wb: &Self::WriteBatch) -> Result<()> { @@ -69,8 +69,8 @@ impl KvEngine for RocksEngine { Self::WriteBatch::new(Arc::clone(&self.0)) } - fn snapshot(&self) -> Snapshot { - Snapshot::new(self.0.clone()) + fn snapshot(&self) -> RocksSnapshot { + RocksSnapshot::new(self.0.clone()) } fn sync(&self) -> Result<()> { @@ -147,7 +147,7 @@ mod tests { use std::sync::Arc; use tempfile::Builder; - use crate::{RocksEngine, Snapshot}; + use crate::{RocksEngine, RocksSnapshot}; #[test] fn test_base() { @@ -264,7 +264,7 @@ mod tests { assert_eq!(data.len(), 1); - let snap = Snapshot::new(engine.get_sync_db()); + let snap = RocksSnapshot::new(engine.get_sync_db()); engine.put(b"a3", b"v3").unwrap(); assert!(engine.seek(b"a3").unwrap().is_some()); diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index d8e20d07b24..5bee751ad96 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -4,7 +4,7 @@ use std::fmt::{self, Debug, Formatter}; use std::ops::Deref; use std::sync::Arc; -use engine_traits::{self, IterOptions, Iterable, Peekable, ReadOptions, Result}; +use engine_traits::{self, IterOptions, Iterable, Peekable, ReadOptions, Result, Snapshot}; use rocksdb::rocksdb_options::UnsafeSnap; use rocksdb::{DBIterator, DB}; @@ -12,27 +12,27 @@ use crate::options::RocksReadOptions; use crate::util::get_cf_handle; use crate::RocksEngineIterator; -pub struct Snapshot { +pub struct RocksSnapshot { // TODO: use &DB. db: Arc, snap: UnsafeSnap, } -unsafe impl Send for Snapshot {} -unsafe impl Sync for Snapshot {} +unsafe impl Send for RocksSnapshot {} +unsafe impl Sync for RocksSnapshot {} -impl Snapshot { +impl RocksSnapshot { pub fn new(db: Arc) -> Self { unsafe { - Snapshot { + RocksSnapshot { snap: db.unsafe_snap(), db, } } } - pub fn into_sync(self) -> SyncSnapshot { - SyncSnapshot(Arc::new(self)) + pub fn into_sync(self) -> RocksSyncSnapshot { + RocksSyncSnapshot(Arc::new(self)) } pub fn get_db(&self) -> &DB { @@ -40,19 +40,19 @@ impl Snapshot { } } -impl engine_traits::Snapshot for Snapshot { +impl Snapshot for RocksSnapshot { fn cf_names(&self) -> Vec<&str> { self.db.cf_names() } } -impl Debug for Snapshot { +impl Debug for RocksSnapshot { fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result { write!(fmt, "Engine Snapshot Impl") } } -impl Drop for Snapshot { +impl Drop for RocksSnapshot { fn drop(&mut self) { unsafe { self.db.release_snap(&self.snap); @@ -60,7 +60,7 @@ impl Drop for Snapshot { } } -impl Iterable for Snapshot { +impl Iterable for RocksSnapshot { type Iterator = RocksEngineIterator; fn iterator_opt(&self, opts: &IterOptions) -> Result { @@ -90,7 +90,7 @@ impl Iterable for Snapshot { } } -impl Peekable for Snapshot { +impl Peekable for RocksSnapshot { fn get_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); @@ -114,18 +114,18 @@ impl Peekable for Snapshot { } #[derive(Clone, Debug)] -pub struct SyncSnapshot(Arc); +pub struct RocksSyncSnapshot(Arc); -impl Deref for SyncSnapshot { - type Target = Snapshot; +impl Deref for RocksSyncSnapshot { + type Target = RocksSnapshot; - fn deref(&self) -> &Snapshot { + fn deref(&self) -> &RocksSnapshot { &self.0 } } -impl SyncSnapshot { - pub fn new(db: Arc) -> SyncSnapshot { - SyncSnapshot(Arc::new(Snapshot::new(db))) +impl RocksSyncSnapshot { + pub fn new(db: Arc) -> RocksSyncSnapshot { + RocksSyncSnapshot(Arc::new(RocksSnapshot::new(db))) } } From efb52a0456dae89e667a75620d52a88dfa440db5 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:21:28 +0000 Subject: [PATCH 15/41] engine_rocks: Add conversion from &RawSnapshot to &RocksSnapshot Signed-off-by: Brian Anderson --- components/engine/src/rocks/snapshot.rs | 1 + components/engine_rocks/src/snapshot.rs | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index 829f1fcfbb5..9b361ecca85 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use super::{CFHandle, DBVector, ReadOptions, UnsafeSnap, DB}; use crate::{DBIterator, Error, IterOption, Iterable, Peekable, Result}; +#[repr(C)] // Guarantee same representation as in engine_rocks pub struct Snapshot { db: Arc, snap: UnsafeSnap, diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index 5bee751ad96..2d8063b0375 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -7,11 +7,13 @@ use std::sync::Arc; use engine_traits::{self, IterOptions, Iterable, Peekable, ReadOptions, Result, Snapshot}; use rocksdb::rocksdb_options::UnsafeSnap; use rocksdb::{DBIterator, DB}; +use engine::rocks::Snapshot as RawSnapshot; use crate::options::RocksReadOptions; use crate::util::get_cf_handle; use crate::RocksEngineIterator; +#[repr(C)] // Guarantee same representation as in engine/rocks pub struct RocksSnapshot { // TODO: use &DB. db: Arc, @@ -31,6 +33,10 @@ impl RocksSnapshot { } } + pub fn from_ref(raw: &RawSnapshot) -> &RocksSnapshot { + unsafe { &*(raw as *const _ as *const _) } + } + pub fn into_sync(self) -> RocksSyncSnapshot { RocksSyncSnapshot(Arc::new(self)) } From b9bfa1a6bce6ccd54f276e9ad5cae254b5bc11e0 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:23:04 +0000 Subject: [PATCH 16/41] engine_rocks: Add static_assertions dep Signed-off-by: Brian Anderson --- Cargo.lock | 7 ++++--- components/engine_rocks/Cargo.toml | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6831723a113..ef06e77b83f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -414,7 +414,7 @@ dependencies = [ "panic_hook", "protobuf 2.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.2", - "static_assertions 1.0.0", + "static_assertions 1.1.0", "tikv_alloc", ] @@ -749,6 +749,7 @@ dependencies = [ "serde_derive", "slog", "slog-global", + "static_assertions 1.1.0", "sys-info", "tempfile", "tikv_alloc", @@ -2753,9 +2754,9 @@ checksum = "c19be23126415861cb3a23e501d34a708f7f9b2183c5252d690941c2e69199d5" [[package]] name = "static_assertions" -version = "1.0.0" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fa13613355688665b68639b1c378a62dbedea78aff0fc59a4fa656cbbdec657" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" [[package]] name = "string" diff --git a/components/engine_rocks/Cargo.toml b/components/engine_rocks/Cargo.toml index 424e4cae569..9b7b9026ad9 100644 --- a/components/engine_rocks/Cargo.toml +++ b/components/engine_rocks/Cargo.toml @@ -18,6 +18,7 @@ crc = "1.8" lazy_static = "1.3" slog = { version = "2.3", features = ["max_level_trace", "release_max_level_debug"] } slog-global = { version = "0.1", git = "https://github.com/breeswish/slog-global.git", rev = "0e23a5baff302a9d7bccd85f8f31e43339c2f2c1" } +static_assertions = "1.1.0" time = "0.1" sys-info = "0.5.7" tikv_alloc = { path = "../tikv_alloc" } From 457bab514c83fc7d6fd202ccb221e195b6c72d2e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:26:39 +0000 Subject: [PATCH 17/41] engine_rocks: Assert some properties of RocksSnapshot Signed-off-by: Brian Anderson --- components/engine_rocks/src/snapshot.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index 2d8063b0375..af1096bc081 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -20,6 +20,9 @@ pub struct RocksSnapshot { snap: UnsafeSnap, } +static_assertions::assert_eq_size!(RocksSnapshot, RawSnapshot); +static_assertions::assert_eq_align!(RocksSnapshot, RawSnapshot); + unsafe impl Send for RocksSnapshot {} unsafe impl Sync for RocksSnapshot {} From 94505720ef1d5cb97b54a778a5e349eb35ed7b0f Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:34:07 +0000 Subject: [PATCH 18/41] engine: Add engine_traits dep Signed-off-by: Brian Anderson --- Cargo.lock | 1 + components/engine/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ef06e77b83f..f0394c88892 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -710,6 +710,7 @@ name = "engine" version = "0.0.1" dependencies = [ "crc", + "engine_traits", "hex", "kvproto", "lazy_static", diff --git a/components/engine/Cargo.toml b/components/engine/Cargo.toml index ae2ae498fcb..0cf8a5c8199 100644 --- a/components/engine/Cargo.toml +++ b/components/engine/Cargo.toml @@ -26,6 +26,7 @@ serde_derive = "1.0" toml = "0.4" hex = "0.3" tikv_util = { path = "../tikv_util" } +engine_traits = { path = "../engine_traits" } [dependencies.prometheus] git = "https://github.com/pingcap/rust-prometheus.git" From f25b5416afe788e16c999f1e6379db7f1bfe6199 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 18:35:56 +0000 Subject: [PATCH 19/41] engine_traits: Move SeekMode into options.rs Signed-off-by: Brian Anderson --- components/engine_traits/src/iterable.rs | 6 ------ components/engine_traits/src/options.rs | 8 ++++++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/components/engine_traits/src/iterable.rs b/components/engine_traits/src/iterable.rs index 287633d919f..e87036782e3 100644 --- a/components/engine_traits/src/iterable.rs +++ b/components/engine_traits/src/iterable.rs @@ -4,12 +4,6 @@ use tikv_util::keybuilder::KeyBuilder; use crate::*; -#[derive(Clone, PartialEq)] -pub enum SeekMode { - TotalOrder, - Prefix, -} - pub enum SeekKey<'a> { Start, End, diff --git a/components/engine_traits/src/options.rs b/components/engine_traits/src/options.rs index 0fce9469186..0483635f470 100644 --- a/components/engine_traits/src/options.rs +++ b/components/engine_traits/src/options.rs @@ -1,8 +1,6 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. use tikv_util::keybuilder::KeyBuilder; -use crate::SeekMode; - #[derive(Clone)] pub struct ReadOptions {} @@ -43,6 +41,12 @@ impl Default for WriteOptions { } } +#[derive(Clone, PartialEq)] +pub enum SeekMode { + TotalOrder, + Prefix, +} + #[derive(Clone)] pub struct IterOptions { pub lower_bound: Option, From 1cdcc7de6d285db57cdcd1cfd0af0c2ea1da6192 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 19:05:17 +0000 Subject: [PATCH 20/41] engine: Move IterOption into engine_traits Eliminates duplicate code. Signed-off-by: Brian Anderson --- components/engine/src/iterable.rs | 133 +++--------------------- components/engine/src/rocks/db.rs | 1 + components/engine/src/rocks/snapshot.rs | 1 + components/engine/src/util.rs | 2 +- components/engine_rocks/src/options.rs | 18 +--- components/engine_traits/src/options.rs | 55 +++++++--- src/server/debug.rs | 1 + 7 files changed, 58 insertions(+), 153 deletions(-) diff --git a/components/engine/src/iterable.rs b/components/engine/src/iterable.rs index af2e026e82e..e036a7665aa 100644 --- a/components/engine/src/iterable.rs +++ b/components/engine/src/iterable.rs @@ -5,141 +5,36 @@ pub use crate::rocks::{DBIterator, ReadOptions, DB}; use crate::Result; use tikv_util::keybuilder::KeyBuilder; -#[derive(Clone, PartialEq)] -enum SeekMode { - TotalOrder, - Prefix, -} +pub use engine_traits::SeekMode; +pub use engine_traits::IterOptions as IterOption; -pub struct IterOption { - lower_bound: Option, - upper_bound: Option, - prefix_same_as_start: bool, - fill_cache: bool, - // only supported when Titan enabled, otherwise it doesn't take effect. - titan_key_only: bool, - seek_mode: SeekMode, +pub trait IterOptionsExt { + fn build_read_opts(self) -> ReadOptions; } -impl IterOption { - pub fn new( - lower_bound: Option, - upper_bound: Option, - fill_cache: bool, - ) -> IterOption { - IterOption { - lower_bound, - upper_bound, - prefix_same_as_start: false, - fill_cache, - titan_key_only: false, - seek_mode: SeekMode::TotalOrder, - } - } - - #[inline] - pub fn use_prefix_seek(mut self) -> IterOption { - self.seek_mode = SeekMode::Prefix; - self - } - - #[inline] - pub fn total_order_seek_used(&self) -> bool { - self.seek_mode == SeekMode::TotalOrder - } - - #[inline] - pub fn fill_cache(&mut self, v: bool) { - self.fill_cache = v; - } - - #[inline] - pub fn titan_key_only(&mut self, v: bool) { - self.titan_key_only = v; - } - - #[inline] - pub fn lower_bound(&self) -> Option<&[u8]> { - self.lower_bound.as_ref().map(|v| v.as_slice()) - } - - #[inline] - pub fn set_lower_bound(&mut self, bound: &[u8], reserved_prefix_len: usize) { - let builder = KeyBuilder::from_slice(bound, reserved_prefix_len, 0); - self.lower_bound = Some(builder); - } - - pub fn set_vec_lower_bound(&mut self, bound: Vec) { - self.lower_bound = Some(KeyBuilder::from_vec(bound, 0, 0)); - } - - pub fn set_lower_bound_prefix(&mut self, prefix: &[u8]) { - if let Some(ref mut builder) = self.lower_bound { - builder.set_prefix(prefix); - } - } - - #[inline] - pub fn upper_bound(&self) -> Option<&[u8]> { - self.upper_bound.as_ref().map(|v| v.as_slice()) - } - - #[inline] - pub fn set_upper_bound(&mut self, bound: &[u8], reserved_prefix_len: usize) { - let builder = KeyBuilder::from_slice(bound, reserved_prefix_len, 0); - self.upper_bound = Some(builder); - } - - pub fn set_vec_upper_bound(&mut self, bound: Vec) { - self.upper_bound = Some(KeyBuilder::from_vec(bound, 0, 0)); - } - - pub fn set_upper_bound_prefix(&mut self, prefix: &[u8]) { - if let Some(ref mut builder) = self.upper_bound { - builder.set_prefix(prefix); - } - } - - #[inline] - pub fn set_prefix_same_as_start(mut self, enable: bool) -> IterOption { - self.prefix_same_as_start = enable; - self - } - - pub fn build_read_opts(self) -> ReadOptions { +impl IterOptionsExt for IterOption { + fn build_read_opts(self) -> ReadOptions { let mut opts = ReadOptions::new(); - opts.fill_cache(self.fill_cache); - if self.titan_key_only { + opts.fill_cache(self.fill_cache()); + if self.titan_key_only() { opts.set_titan_key_only(true); } if self.total_order_seek_used() { opts.set_total_order_seek(true); - } else if self.prefix_same_as_start { + } else if self.prefix_same_as_start() { opts.set_prefix_same_as_start(true); } - if let Some(builder) = self.lower_bound { - opts.set_iterate_lower_bound(builder.build()); + let (lower, upper) = self.build_bounds(); + if let Some(lower) = lower { + opts.set_iterate_lower_bound(lower); } - if let Some(builder) = self.upper_bound { - opts.set_iterate_upper_bound(builder.build()); + if let Some(upper) = upper { + opts.set_iterate_upper_bound(upper); } opts } } -impl Default for IterOption { - fn default() -> IterOption { - IterOption { - lower_bound: None, - upper_bound: None, - prefix_same_as_start: false, - fill_cache: true, - titan_key_only: false, - seek_mode: SeekMode::TotalOrder, - } - } -} - // TODO: refactor this trait into rocksdb trait. pub trait Iterable { fn new_iterator(&self, iter_opt: IterOption) -> DBIterator<&DB>; diff --git a/components/engine/src/rocks/db.rs b/components/engine/src/rocks/db.rs index dd2c6b0f1c2..0851dc6934a 100644 --- a/components/engine/src/rocks/db.rs +++ b/components/engine/src/rocks/db.rs @@ -4,6 +4,7 @@ use std::option::Option; use super::{util, DBIterator, DBVector, WriteBatch, DB}; use crate::{IterOption, Iterable, Mutable, Peekable, Result}; +use crate::iterable::IterOptionsExt; impl Peekable for DB { fn get_value(&self, key: &[u8]) -> Result> { diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index 9b361ecca85..2cf27d26fbf 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use super::{CFHandle, DBVector, ReadOptions, UnsafeSnap, DB}; use crate::{DBIterator, Error, IterOption, Iterable, Peekable, Result}; +use crate::iterable::IterOptionsExt; #[repr(C)] // Guarantee same representation as in engine_rocks pub struct Snapshot { diff --git a/components/engine/src/util.rs b/components/engine/src/util.rs index 0bbe6ff5423..4cf7e57ff18 100644 --- a/components/engine/src/util.rs +++ b/components/engine/src/util.rs @@ -47,7 +47,7 @@ pub fn delete_all_in_range_cf( if db.is_titan() { // Cause DeleteFilesInRange may expose old blob index keys, setting key only for Titan // to avoid referring to missing blob files. - iter_opt.titan_key_only(true); + iter_opt.set_titan_key_only(true); } let mut it = db.new_iterator_cf(cf, iter_opt)?; it.seek(start_key.into()); diff --git a/components/engine_rocks/src/options.rs b/components/engine_rocks/src/options.rs index 6f1cc51da82..e25001b762c 100644 --- a/components/engine_rocks/src/options.rs +++ b/components/engine_rocks/src/options.rs @@ -1,6 +1,7 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. use rocksdb::{ReadOptions as RawReadOptions, WriteOptions as RawWriteOptions}; +use engine::IterOptionsExt; pub struct RocksReadOptions(RawReadOptions); @@ -46,22 +47,7 @@ impl From<&engine_traits::WriteOptions> for RocksWriteOptions { impl From for RocksReadOptions { fn from(opts: engine_traits::IterOptions) -> Self { - let mut r = RawReadOptions::default(); - r.fill_cache(opts.fill_cache()); - if opts.key_only() { - r.set_titan_key_only(true); - } - if opts.total_order_seek_used() { - r.set_total_order_seek(true); - } else if opts.prefix_same_as_start() { - r.set_prefix_same_as_start(true); - } - if let Some(builder) = opts.lower_bound { - r.set_iterate_lower_bound(builder.build()); - } - if let Some(builder) = opts.upper_bound { - r.set_iterate_upper_bound(builder.build()); - } + let r = opts.build_read_opts(); RocksReadOptions(r) } } diff --git a/components/engine_traits/src/options.rs b/components/engine_traits/src/options.rs index 0483635f470..9f4e2e7ac62 100644 --- a/components/engine_traits/src/options.rs +++ b/components/engine_traits/src/options.rs @@ -47,13 +47,13 @@ pub enum SeekMode { Prefix, } -#[derive(Clone)] pub struct IterOptions { - pub lower_bound: Option, - pub upper_bound: Option, + lower_bound: Option, + upper_bound: Option, prefix_same_as_start: bool, fill_cache: bool, - key_only: bool, + // only supported when Titan enabled, otherwise it doesn't take effect. + titan_key_only: bool, seek_mode: SeekMode, } @@ -68,40 +68,48 @@ impl IterOptions { upper_bound, prefix_same_as_start: false, fill_cache, - key_only: false, + titan_key_only: false, seek_mode: SeekMode::TotalOrder, } } + #[inline] pub fn use_prefix_seek(mut self) -> IterOptions { self.seek_mode = SeekMode::Prefix; self } + #[inline] pub fn total_order_seek_used(&self) -> bool { self.seek_mode == SeekMode::TotalOrder } - pub fn set_fill_cache(&mut self, v: bool) { - self.fill_cache = v; - } - + #[inline] pub fn fill_cache(&self) -> bool { self.fill_cache } - pub fn set_key_only(&mut self, v: bool) { - self.key_only = v; + #[inline] + pub fn set_fill_cache(&mut self, v: bool) { + self.fill_cache = v; } - pub fn key_only(&self) -> bool { - self.key_only + #[inline] + pub fn titan_key_only(&self) -> bool { + self.titan_key_only } + #[inline] + pub fn set_titan_key_only(&mut self, v: bool) { + self.titan_key_only = v; + } + + #[inline] pub fn lower_bound(&self) -> Option<&[u8]> { self.lower_bound.as_ref().map(|v| v.as_slice()) } + #[inline] pub fn set_lower_bound(&mut self, bound: &[u8], reserved_prefix_len: usize) { let builder = KeyBuilder::from_slice(bound, reserved_prefix_len, 0); self.lower_bound = Some(builder); @@ -117,10 +125,12 @@ impl IterOptions { } } + #[inline] pub fn upper_bound(&self) -> Option<&[u8]> { self.upper_bound.as_ref().map(|v| v.as_slice()) } + #[inline] pub fn set_upper_bound(&mut self, bound: &[u8], reserved_prefix_len: usize) { let builder = KeyBuilder::from_slice(bound, reserved_prefix_len, 0); self.upper_bound = Some(builder); @@ -136,13 +146,23 @@ impl IterOptions { } } - pub fn set_prefix_same_as_start(&mut self, enable: bool) { - self.prefix_same_as_start = enable; + #[inline] + pub fn build_bounds(self) -> (Option>, Option>) { + let lower = self.lower_bound.map(KeyBuilder::build); + let upper = self.upper_bound.map(KeyBuilder::build); + (lower, upper) } + #[inline] pub fn prefix_same_as_start(&self) -> bool { self.prefix_same_as_start } + + #[inline] + pub fn set_prefix_same_as_start(mut self, enable: bool) -> IterOptions { + self.prefix_same_as_start = enable; + self + } } impl Default for IterOptions { @@ -151,9 +171,10 @@ impl Default for IterOptions { lower_bound: None, upper_bound: None, prefix_same_as_start: false, - fill_cache: false, - key_only: false, + fill_cache: true, + titan_key_only: false, seek_mode: SeekMode::TotalOrder, } } } + diff --git a/src/server/debug.rs b/src/server/debug.rs index 18d810037d1..68de2cfd8b6 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -14,6 +14,7 @@ use engine::rocks::{ }; use engine::{self, Engines, IterOption, Iterable, Mutable, Peekable}; use engine::{CF_DEFAULT, CF_LOCK, CF_RAFT, CF_WRITE}; +use engine::IterOptionsExt; use kvproto::debugpb::{self, Db as DBType, Module}; use kvproto::kvrpcpb::{MvccInfo, MvccLock, MvccValue, MvccWrite, Op}; use kvproto::metapb::{Peer, Region}; From 4d5f8a3f286071f6a5686f697e26a2e1942b7361 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Mon, 11 Nov 2019 19:09:22 +0000 Subject: [PATCH 21/41] engine_traits: Add Iterable bound to Snapshot Signed-off-by: Brian Anderson --- components/engine_traits/src/snapshot.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine_traits/src/snapshot.rs b/components/engine_traits/src/snapshot.rs index 174e903abd5..6026cb52bb5 100644 --- a/components/engine_traits/src/snapshot.rs +++ b/components/engine_traits/src/snapshot.rs @@ -1,8 +1,9 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. +use crate::iterable::Iterable; use crate::peekable::Peekable; use std::fmt::Debug; -pub trait Snapshot: 'static + Peekable + Send + Sync + Debug { +pub trait Snapshot: 'static + Peekable + Iterable + Send + Sync + Debug { fn cf_names(&self) -> Vec<&str>; } From 15de487bd81b09363d5b0392e0c750c19789742d Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 12 Nov 2019 13:48:46 +0100 Subject: [PATCH 22/41] engine_rocks: Add Compat implementation for snapshots Signed-off-by: Brian Anderson --- components/engine_rocks/src/compat.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/components/engine_rocks/src/compat.rs b/components/engine_rocks/src/compat.rs index 9ee9c0fbf77..726b301d58c 100644 --- a/components/engine_rocks/src/compat.rs +++ b/components/engine_rocks/src/compat.rs @@ -3,19 +3,34 @@ use crate::engine::RocksEngine; use engine::DB; use std::sync::Arc; +use engine::Snapshot as RawSnapshot; +use crate::snapshot::RocksSnapshot; /// A trait to enter the world of engine traits from a raw `Arc` /// with as little syntax as possible. /// /// This will be used during the transition from RocksDB to the /// `KvEngine` abstraction and then discarded. -pub trait EngineCompat { - fn c(&self) -> &RocksEngine; +pub trait Compat { + type Other; + + fn c(&self) -> &Self::Other; } -impl EngineCompat for Arc { +impl Compat for Arc { + type Other = RocksEngine; + #[inline] fn c(&self) -> &RocksEngine { RocksEngine::from_ref(self) } } + +impl Compat for RawSnapshot { + type Other = RocksSnapshot; + + #[inline] + fn c(&self) -> &RocksSnapshot { + RocksSnapshot::from_ref(self) + } +} From 6fe421ce44cc7e74e45658a527a10a0defdc5168 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 12 Nov 2019 14:02:17 +0100 Subject: [PATCH 23/41] engine_traits: Add From<&[u8]> for SeekKey Signed-off-by: Brian Anderson --- components/engine_traits/src/iterable.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/engine_traits/src/iterable.rs b/components/engine_traits/src/iterable.rs index e87036782e3..0040a01937a 100644 --- a/components/engine_traits/src/iterable.rs +++ b/components/engine_traits/src/iterable.rs @@ -137,3 +137,9 @@ impl<'a, I: Iterator> std::iter::Iterator for StdIterator<'a, I> { kv } } + +impl<'a> From<&'a [u8]> for SeekKey<'a> { + fn from(bs: &'a [u8]) -> SeekKey { + SeekKey::Key(bs) + } +} From f46bdf4479918fc028575befe7b0209c21cb52be Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 13:28:13 -0500 Subject: [PATCH 24/41] engine_traits: Remove Result return from key/value methods The underlying engine doesn't require this, and it just makes conversion harder. Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine_iterator.rs | 8 ++++---- components/engine_rocks/src/sst.rs | 8 ++++---- components/engine_traits/src/iterable.rs | 14 +++++++------- components/sst_importer/src/sst_importer.rs | 8 ++++---- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/components/engine_rocks/src/engine_iterator.rs b/components/engine_rocks/src/engine_iterator.rs index d61b5835b7d..cce2a16297a 100644 --- a/components/engine_rocks/src/engine_iterator.rs +++ b/components/engine_rocks/src/engine_iterator.rs @@ -34,12 +34,12 @@ impl engine_traits::Iterator for RocksEngineIterator { self.0.next() } - fn key(&self) -> Result<&[u8]> { - Ok(self.0.key()) + fn key(&self) -> &[u8] { + self.0.key() } - fn value(&self) -> Result<&[u8]> { - Ok(self.0.value()) + fn value(&self) -> &[u8] { + self.0.value() } fn valid(&self) -> bool { diff --git a/components/engine_rocks/src/sst.rs b/components/engine_rocks/src/sst.rs index bd97fbdfe02..9ed69d4a686 100644 --- a/components/engine_rocks/src/sst.rs +++ b/components/engine_rocks/src/sst.rs @@ -90,12 +90,12 @@ impl Iterator for RocksSstIterator { self.0.next() } - fn key(&self) -> Result<&[u8]> { - Ok(self.0.key()) + fn key(&self) -> &[u8] { + self.0.key() } - fn value(&self) -> Result<&[u8]> { - Ok(self.0.value()) + fn value(&self) -> &[u8] { + self.0.value() } fn valid(&self) -> bool { diff --git a/components/engine_traits/src/iterable.rs b/components/engine_traits/src/iterable.rs index 0040a01937a..0da20676a3a 100644 --- a/components/engine_traits/src/iterable.rs +++ b/components/engine_traits/src/iterable.rs @@ -17,13 +17,13 @@ pub trait Iterator { fn prev(&mut self) -> bool; fn next(&mut self) -> bool; - fn key(&self) -> Result<&[u8]>; - fn value(&self) -> Result<&[u8]>; + fn key(&self) -> &[u8]; + fn value(&self) -> &[u8]; fn kv(&self) -> Option<(Vec, Vec)> { if self.valid() { - let k = self.key().ok()?; - let v = self.value().ok()?; + let k = self.key(); + let v = self.value(); Some((k.to_vec(), v.to_vec())) } else { None @@ -88,7 +88,7 @@ pub trait Iterable { let mut iter = self.iterator()?; iter.seek(SeekKey::Key(key)); if iter.valid() { - Ok(Some((iter.key()?.to_vec(), iter.value()?.to_vec()))) + Ok(Some((iter.key().to_vec(), iter.value().to_vec()))) } else { Ok(None) } @@ -99,7 +99,7 @@ pub trait Iterable { let mut iter = self.iterator_cf(cf)?; iter.seek(SeekKey::Key(key)); if iter.valid() { - Ok(Some((iter.key()?.to_vec(), iter.value()?.to_vec()))) + Ok(Some((iter.key().to_vec(), iter.value().to_vec()))) } else { Ok(None) } @@ -113,7 +113,7 @@ where { it.seek(SeekKey::Key(start_key)); while it.valid() { - let r = f(it.key()?, it.value()?)?; + let r = f(it.key(), it.value())?; if !r || !it.next() { break; } diff --git a/components/sst_importer/src/sst_importer.rs b/components/sst_importer/src/sst_importer.rs index 94f538cecff..c4ed141dcac 100644 --- a/components/sst_importer/src/sst_importer.rs +++ b/components/sst_importer/src/sst_importer.rs @@ -189,7 +189,7 @@ impl SSTImporter { // the SST is empty, so no need to iterate at all (should be impossible?) return Ok(Some(meta.get_range().clone())); } - let start_key = keys::origin_key(iter.key()?); + let start_key = keys::origin_key(iter.key()); if is_before_start_bound(start_key, &range_start) { // SST's start is before the range to consume, so needs to iterate to skip over return Ok(None); @@ -198,7 +198,7 @@ impl SSTImporter { // seek to end and fetch the last (inclusive) key of the SST. iter.seek(SeekKey::End); - let last_key = keys::origin_key(iter.key()?); + let last_key = keys::origin_key(iter.key()); if is_after_end_bound(last_key, &range_end) { // SST's end is after the range to consume return Ok(None); @@ -229,7 +229,7 @@ impl SSTImporter { Bound::Excluded(_) => unreachable!(), }; while iter.valid() { - let old_key = keys::origin_key(iter.key()?); + let old_key = keys::origin_key(iter.key()); if is_after_end_bound(old_key, &range_end) { break; } @@ -243,7 +243,7 @@ impl SSTImporter { key.truncate(new_prefix_data_key_len); key.extend_from_slice(&old_key[old_prefix.len()..]); - sst_writer.put(&key, iter.value()?)?; + sst_writer.put(&key, iter.value())?; iter.next(); if first_key.is_none() { first_key = Some(keys::origin_key(&key).to_vec()); From 8b6d9550ea71e783644e862bf11fadd6f372080a Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 14:58:42 -0500 Subject: [PATCH 25/41] engine_traits: Take IterOptions by value The underlying implementation needs to consume this type. By-value is the way the existing API works, so this is easier to migrate. Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine.rs | 4 ++-- components/engine_rocks/src/options.rs | 6 ------ components/engine_rocks/src/snapshot.rs | 4 ++-- components/engine_rocks/src/sst.rs | 4 ++-- components/engine_traits/src/iterable.rs | 12 ++++++------ 5 files changed, 12 insertions(+), 18 deletions(-) diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index 0b8e34da320..93c3cb0eb2d 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -85,7 +85,7 @@ impl KvEngine for RocksEngine { impl Iterable for RocksEngine { type Iterator = RocksEngineIterator; - fn iterator_opt(&self, opts: &IterOptions) -> Result { + fn iterator_opt(&self, opts: IterOptions) -> Result { let opt: RocksReadOptions = opts.into(); Ok(RocksEngineIterator::from_raw(DBIterator::new( self.0.clone(), @@ -93,7 +93,7 @@ impl Iterable for RocksEngine { ))) } - fn iterator_cf_opt(&self, opts: &IterOptions, cf: &str) -> Result { + fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result { let handle = get_cf_handle(&self.0, cf)?; let opt: RocksReadOptions = opts.into(); Ok(RocksEngineIterator::from_raw(DBIterator::new_cf( diff --git a/components/engine_rocks/src/options.rs b/components/engine_rocks/src/options.rs index e25001b762c..c13d4bc03c7 100644 --- a/components/engine_rocks/src/options.rs +++ b/components/engine_rocks/src/options.rs @@ -51,9 +51,3 @@ impl From for RocksReadOptions { RocksReadOptions(r) } } - -impl From<&engine_traits::IterOptions> for RocksReadOptions { - fn from(opts: &engine_traits::IterOptions) -> Self { - opts.clone().into() - } -} diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index af1096bc081..03fef91dfce 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -72,7 +72,7 @@ impl Drop for RocksSnapshot { impl Iterable for RocksSnapshot { type Iterator = RocksEngineIterator; - fn iterator_opt(&self, opts: &IterOptions) -> Result { + fn iterator_opt(&self, opts: IterOptions) -> Result { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { @@ -84,7 +84,7 @@ impl Iterable for RocksSnapshot { ))) } - fn iterator_cf_opt(&self, opts: &IterOptions, cf: &str) -> Result { + fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { diff --git a/components/engine_rocks/src/sst.rs b/components/engine_rocks/src/sst.rs index 9ed69d4a686..8ab2408b64d 100644 --- a/components/engine_rocks/src/sst.rs +++ b/components/engine_rocks/src/sst.rs @@ -54,7 +54,7 @@ impl SstReader for RocksSstReader { impl Iterable for RocksSstReader { type Iterator = RocksSstIterator; - fn iterator_opt(&self, opts: &IterOptions) -> Result { + fn iterator_opt(&self, opts: IterOptions) -> Result { let opt: RocksReadOptions = opts.into(); let opt = opt.into_raw(); Ok(RocksSstIterator(SstFileReader::iter_opt_rc( @@ -63,7 +63,7 @@ impl Iterable for RocksSstReader { ))) } - fn iterator_cf_opt(&self, _opts: &IterOptions, _cf: &str) -> Result { + fn iterator_cf_opt(&self, _opts: IterOptions, _cf: &str) -> Result { unimplemented!() // FIXME: What should happen here? } } diff --git a/components/engine_traits/src/iterable.rs b/components/engine_traits/src/iterable.rs index 0da20676a3a..a0c1812ca87 100644 --- a/components/engine_traits/src/iterable.rs +++ b/components/engine_traits/src/iterable.rs @@ -44,15 +44,15 @@ pub trait Iterator { pub trait Iterable { type Iterator: Iterator; - fn iterator_opt(&self, opts: &IterOptions) -> Result; - fn iterator_cf_opt(&self, opts: &IterOptions, cf: &str) -> Result; + fn iterator_opt(&self, opts: IterOptions) -> Result; + fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result; fn iterator(&self) -> Result { - self.iterator_opt(&IterOptions::default()) + self.iterator_opt(IterOptions::default()) } fn iterator_cf(&self, cf: &str) -> Result { - self.iterator_cf_opt(&IterOptions::default(), cf) + self.iterator_cf_opt(IterOptions::default(), cf) } fn scan(&self, start_key: &[u8], end_key: &[u8], fill_cache: bool, f: F) -> Result<()> @@ -62,7 +62,7 @@ pub trait Iterable { let start = KeyBuilder::from_slice(start_key, DATA_KEY_PREFIX_LEN, 0); let end = KeyBuilder::from_slice(end_key, DATA_KEY_PREFIX_LEN, 0); let iter_opt = IterOptions::new(Some(start), Some(end), fill_cache); - scan_impl(self.iterator_opt(&iter_opt)?, start_key, f) + scan_impl(self.iterator_opt(iter_opt)?, start_key, f) } // like `scan`, only on a specific column family. @@ -80,7 +80,7 @@ pub trait Iterable { let start = KeyBuilder::from_slice(start_key, DATA_KEY_PREFIX_LEN, 0); let end = KeyBuilder::from_slice(end_key, DATA_KEY_PREFIX_LEN, 0); let iter_opt = IterOptions::new(Some(start), Some(end), fill_cache); - scan_impl(self.iterator_cf_opt(&iter_opt, cf)?, start_key, f) + scan_impl(self.iterator_cf_opt(iter_opt, cf)?, start_key, f) } // Seek the first key >= given key, if not found, return None. From 6f309ca9b880531b099cd33927b1c87f28e4bc1e Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 15:46:19 -0500 Subject: [PATCH 26/41] engine: Migrate Snapshot::db_iterator to engine_traits Signed-off-by: Brian Anderson --- components/engine/src/rocks/snapshot.rs | 8 ---- src/raftstore/store/region_snapshot.rs | 14 ++++-- src/server/debug.rs | 2 +- src/storage/kv/into_protoerror.rs | 24 ++++++++++ src/storage/kv/mod.rs | 8 ++++ src/storage/kv/rocksdb_engine.rs | 63 ++++++++++++++++++++++--- 6 files changed, 98 insertions(+), 21 deletions(-) create mode 100644 src/storage/kv/into_protoerror.rs diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index 2cf27d26fbf..dbe272b477c 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -46,14 +46,6 @@ impl Snapshot { Arc::clone(&self.db) } - pub fn db_iterator(&self, iter_opt: IterOption) -> DBIterator> { - let mut opt = iter_opt.build_read_opts(); - unsafe { - opt.set_snapshot(&self.snap); - } - DBIterator::new(Arc::clone(&self.db), opt) - } - pub fn db_iterator_cf(&self, cf: &str, iter_opt: IterOption) -> Result>> { let handle = super::util::get_cf_handle(&self.db, cf)?; let mut opt = iter_opt.build_read_opts(); diff --git a/src/raftstore/store/region_snapshot.rs b/src/raftstore/store/region_snapshot.rs index 98eeb64325b..eeaa05e2a31 100644 --- a/src/raftstore/store/region_snapshot.rs +++ b/src/raftstore/store/region_snapshot.rs @@ -1,16 +1,19 @@ // Copyright 2016 TiKV Project Authors. Licensed under Apache-2.0. -use engine::rocks::{DBIterator, DBVector, SeekKey, TablePropertiesCollection, DB}; +use engine::rocks::{DBVector, TablePropertiesCollection, DB}; use engine::{ self, Error as EngineError, IterOption, Peekable, Result as EngineResult, Snapshot, SyncSnapshot, }; +use engine_traits::SeekKey; +use engine_rocks::{Compat, RocksEngineIterator}; use kvproto::metapb::Region; use std::sync::Arc; use crate::raftstore::store::keys::DATA_PREFIX_KEY; use crate::raftstore::store::{keys, util, PeerStorage}; use crate::raftstore::Result; +use engine_traits::{Iterable, Iterator}; use engine_traits::util::check_key_in_range; use tikv_util::keybuilder::KeyBuilder; use tikv_util::metrics::CRITICAL_ERROR; @@ -200,7 +203,7 @@ impl Peekable for RegionSnapshot { /// iterate in the region. It behaves as if underlying /// db only contains one region. pub struct RegionIterator { - iter: DBIterator>, + iter: RocksEngineIterator, valid: bool, region: Arc, start_key: Vec, @@ -238,7 +241,8 @@ impl RegionIterator { update_upper_bound(&mut iter_opt, ®ion); let start_key = iter_opt.lower_bound().unwrap().to_vec(); let end_key = iter_opt.upper_bound().unwrap().to_vec(); - let iter = snap.db_iterator(iter_opt); + let iter = snap.c().iterator_opt(iter_opt) + .expect("creating snapshot iterator"); // FIXME error handling RegionIterator { iter, valid: false, @@ -258,7 +262,8 @@ impl RegionIterator { update_upper_bound(&mut iter_opt, ®ion); let start_key = iter_opt.lower_bound().unwrap().to_vec(); let end_key = iter_opt.upper_bound().unwrap().to_vec(); - let iter = snap.db_iterator_cf(cf, iter_opt).unwrap(); + let iter = snap.c().iterator_cf_opt(iter_opt, cf) + .expect("creating snapshot iterator"); // FIXME error handling RegionIterator { iter, valid: false, @@ -364,7 +369,6 @@ impl RegionIterator { pub fn status(&self) -> Result<()> { self.iter .status() - .map_err(|e| EngineError::RocksDb(e)) .map_err(From::from) } diff --git a/src/server/debug.rs b/src/server/debug.rs index 68de2cfd8b6..ecc536ab0f3 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -35,7 +35,7 @@ use crate::raftstore::store::{ use crate::raftstore::store::{keys, PeerStorage}; use crate::storage::mvcc::{Lock, LockType, Write, WriteType}; use crate::storage::types::Key; -use crate::storage::Iterator as EngineIterator; +use crate::storage::kv::Iterator as EngineIterator; use tikv_util::codec::bytes; use tikv_util::collections::HashSet; use tikv_util::config::ReadableSize; diff --git a/src/storage/kv/into_protoerror.rs b/src/storage/kv/into_protoerror.rs new file mode 100644 index 00000000000..03d15dfeddd --- /dev/null +++ b/src/storage/kv/into_protoerror.rs @@ -0,0 +1,24 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +use kvproto::errorpb::Error as ProtoError; +use engine_traits::Error as EngineTraitsError; + +pub trait IntoProtoError { + fn into_protoerror(self) -> ProtoError; +} + +impl IntoProtoError for EngineTraitsError { + fn into_protoerror(self) -> ProtoError { + let mut errorpb = ProtoError::default(); + errorpb.set_message(format!("{}", self)); + + if let EngineTraitsError::NotInRange(key, region_id, start_key, end_key) = self { + errorpb.mut_key_not_in_region().set_key(key); + errorpb.mut_key_not_in_region().set_region_id(region_id); + errorpb.mut_key_not_in_region().set_start_key(start_key); + errorpb.mut_key_not_in_region().set_end_key(end_key); + } + + errorpb + } +} diff --git a/src/storage/kv/mod.rs b/src/storage/kv/mod.rs index 4360ea86d63..c86228f8a05 100644 --- a/src/storage/kv/mod.rs +++ b/src/storage/kv/mod.rs @@ -12,6 +12,7 @@ use kvproto::kvrpcpb::Context; use crate::raftstore::coprocessor::SeekRegionCallback; use crate::storage::{Key, Value}; +use self::into_protoerror::IntoProtoError; mod btree_engine; mod compact_listener; @@ -19,6 +20,7 @@ mod cursor; mod perf_context; mod rocksdb_engine; mod stats; +mod into_protoerror; pub use self::btree_engine::{BTreeEngine, BTreeEngineIterator, BTreeEngineSnapshot}; pub use self::compact_listener::{CompactedEvent, CompactionListener}; @@ -185,6 +187,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: engine_traits::Error) -> Error { + Error::Request(err.into_protoerror()) + } +} + impl Error { pub fn maybe_clone(&self) -> Option { match *self { diff --git a/src/storage/kv/rocksdb_engine.rs b/src/storage/kv/rocksdb_engine.rs index 3d08a91c7ae..58b6f173683 100644 --- a/src/storage/kv/rocksdb_engine.rs +++ b/src/storage/kv/rocksdb_engine.rs @@ -9,11 +9,13 @@ use std::time::Duration; use engine::rocks; use engine::rocks::util::CFOptions; -use engine::rocks::{ColumnFamilyOptions, DBIterator, SeekKey, Writable, WriteBatch, DB}; +use engine::rocks::{ColumnFamilyOptions, DBIterator, SeekKey as DBSeekKey, Writable, WriteBatch, DB}; use engine::Engines; use engine::Error as EngineError; use engine::{CfName, CF_DEFAULT, CF_LOCK, CF_RAFT, CF_WRITE}; use engine::{IterOption, Peekable}; +use engine_traits::{Iterable, Iterator, SeekKey}; +use engine_rocks::{Compat, RocksEngineIterator}; use kvproto::kvrpcpb::Context; use tempfile::{Builder, TempDir}; @@ -291,7 +293,7 @@ impl Engine for RocksEngine { } impl Snapshot for RocksSnapshot { - type Iter = DBIterator>; + type Iter = RocksEngineIterator; fn get(&self, key: &Key) -> Result> { trace!("RocksSnapshot: get"; "key" => %key); @@ -307,7 +309,7 @@ impl Snapshot for RocksSnapshot { fn iter(&self, iter_opt: IterOption, mode: ScanMode) -> Result> { trace!("RocksSnapshot: create iterator"); - let iter = self.db_iterator(iter_opt); + let iter = self.c().iterator_opt(iter_opt)?; Ok(Cursor::new(iter, mode)) } @@ -318,11 +320,57 @@ impl Snapshot for RocksSnapshot { mode: ScanMode, ) -> Result> { trace!("RocksSnapshot: create cf iterator"); - let iter = self.db_iterator_cf(cf, iter_opt)?; + let iter = self.c().iterator_cf_opt(iter_opt, cf)?; Ok(Cursor::new(iter, mode)) } } +impl EngineIterator for RocksEngineIterator { + fn next(&mut self) -> bool { + Iterator::next(self) + } + + fn prev(&mut self) -> bool { + Iterator::prev(self) + } + + fn seek(&mut self, key: &Key) -> Result { + Ok(Iterator::seek(self, key.as_encoded().as_slice().into())) + } + + fn seek_for_prev(&mut self, key: &Key) -> Result { + Ok(Iterator::seek_for_prev( + self, + key.as_encoded().as_slice().into(), + )) + } + + fn seek_to_first(&mut self) -> bool { + Iterator::seek(self, SeekKey::Start) + } + + fn seek_to_last(&mut self) -> bool { + Iterator::seek(self, SeekKey::End) + } + + fn valid(&self) -> bool { + Iterator::valid(self) + } + + fn status(&self) -> Result<()> { + Iterator::status(self) + .map_err(From::from) + } + + fn key(&self) -> &[u8] { + Iterator::key(self) + } + + fn value(&self) -> &[u8] { + Iterator::value(self) + } +} + impl + Send> EngineIterator for DBIterator { fn next(&mut self) -> bool { DBIterator::next(self) @@ -344,11 +392,11 @@ impl + Send> EngineIterator for DBIterator { } fn seek_to_first(&mut self) -> bool { - DBIterator::seek(self, SeekKey::Start) + DBIterator::seek(self, DBSeekKey::Start) } fn seek_to_last(&mut self) -> bool { - DBIterator::seek(self, SeekKey::End) + DBIterator::seek(self, DBSeekKey::End) } fn valid(&self) -> bool { @@ -358,7 +406,7 @@ impl + Send> EngineIterator for DBIterator { fn status(&self) -> Result<()> { DBIterator::status(self) .map_err(|e| EngineError::RocksDb(e)) - .map_err(From::from) + .map_err(Error::from) } fn key(&self) -> &[u8] { @@ -370,6 +418,7 @@ impl + Send> EngineIterator for DBIterator { } } + #[cfg(test)] mod tests { pub use super::super::perf_context::{PerfStatisticsDelta, PerfStatisticsInstant}; From 34f46d8cd544f0ebef0cfe83526ad0934ac51509 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 16:30:31 -0500 Subject: [PATCH 27/41] engine: Remove unused db_iterator_cf method Signed-off-by: Brian Anderson --- components/engine/src/rocks/snapshot.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index dbe272b477c..d853bb95046 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -45,15 +45,6 @@ impl Snapshot { pub fn get_db(&self) -> Arc { Arc::clone(&self.db) } - - pub fn db_iterator_cf(&self, cf: &str, iter_opt: IterOption) -> Result>> { - let handle = super::util::get_cf_handle(&self.db, cf)?; - let mut opt = iter_opt.build_read_opts(); - unsafe { - opt.set_snapshot(&self.snap); - } - Ok(DBIterator::new_cf(Arc::clone(&self.db), handle, opt)) - } } impl Debug for Snapshot { From 06f26333e8c905ee0e464bd3c5f8c8c4d4b25f22 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 16:45:05 -0500 Subject: [PATCH 28/41] engine_traits: Reorder arguments to iterator_cf_opt Consistent with other trait methods and existing engine methods, cf comes first in the argument list. Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine.rs | 2 +- components/engine_rocks/src/snapshot.rs | 2 +- components/engine_rocks/src/sst.rs | 2 +- components/engine_traits/src/iterable.rs | 6 +++--- src/raftstore/store/region_snapshot.rs | 2 +- src/storage/kv/rocksdb_engine.rs | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index 93c3cb0eb2d..aaf4672d36b 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -93,7 +93,7 @@ impl Iterable for RocksEngine { ))) } - fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result { + fn iterator_cf_opt(&self, cf: &str, opts: IterOptions) -> Result { let handle = get_cf_handle(&self.0, cf)?; let opt: RocksReadOptions = opts.into(); Ok(RocksEngineIterator::from_raw(DBIterator::new_cf( diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index 03fef91dfce..29b7cde550f 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -84,7 +84,7 @@ impl Iterable for RocksSnapshot { ))) } - fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result { + fn iterator_cf_opt(&self, cf: &str, opts: IterOptions) -> Result { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { diff --git a/components/engine_rocks/src/sst.rs b/components/engine_rocks/src/sst.rs index 8ab2408b64d..bc02569bda1 100644 --- a/components/engine_rocks/src/sst.rs +++ b/components/engine_rocks/src/sst.rs @@ -63,7 +63,7 @@ impl Iterable for RocksSstReader { ))) } - fn iterator_cf_opt(&self, _opts: IterOptions, _cf: &str) -> Result { + fn iterator_cf_opt(&self, _cf: &str, _opts: IterOptions) -> Result { unimplemented!() // FIXME: What should happen here? } } diff --git a/components/engine_traits/src/iterable.rs b/components/engine_traits/src/iterable.rs index a0c1812ca87..788ab9cb568 100644 --- a/components/engine_traits/src/iterable.rs +++ b/components/engine_traits/src/iterable.rs @@ -45,14 +45,14 @@ pub trait Iterable { type Iterator: Iterator; fn iterator_opt(&self, opts: IterOptions) -> Result; - fn iterator_cf_opt(&self, opts: IterOptions, cf: &str) -> Result; + fn iterator_cf_opt(&self, cf: &str, opts: IterOptions) -> Result; fn iterator(&self) -> Result { self.iterator_opt(IterOptions::default()) } fn iterator_cf(&self, cf: &str) -> Result { - self.iterator_cf_opt(IterOptions::default(), cf) + self.iterator_cf_opt(cf, IterOptions::default()) } fn scan(&self, start_key: &[u8], end_key: &[u8], fill_cache: bool, f: F) -> Result<()> @@ -80,7 +80,7 @@ pub trait Iterable { let start = KeyBuilder::from_slice(start_key, DATA_KEY_PREFIX_LEN, 0); let end = KeyBuilder::from_slice(end_key, DATA_KEY_PREFIX_LEN, 0); let iter_opt = IterOptions::new(Some(start), Some(end), fill_cache); - scan_impl(self.iterator_cf_opt(iter_opt, cf)?, start_key, f) + scan_impl(self.iterator_cf_opt(cf, iter_opt)?, start_key, f) } // Seek the first key >= given key, if not found, return None. diff --git a/src/raftstore/store/region_snapshot.rs b/src/raftstore/store/region_snapshot.rs index eeaa05e2a31..72a906667f5 100644 --- a/src/raftstore/store/region_snapshot.rs +++ b/src/raftstore/store/region_snapshot.rs @@ -262,7 +262,7 @@ impl RegionIterator { update_upper_bound(&mut iter_opt, ®ion); let start_key = iter_opt.lower_bound().unwrap().to_vec(); let end_key = iter_opt.upper_bound().unwrap().to_vec(); - let iter = snap.c().iterator_cf_opt(iter_opt, cf) + let iter = snap.c().iterator_cf_opt(cf, iter_opt) .expect("creating snapshot iterator"); // FIXME error handling RegionIterator { iter, diff --git a/src/storage/kv/rocksdb_engine.rs b/src/storage/kv/rocksdb_engine.rs index 58b6f173683..af5496117ca 100644 --- a/src/storage/kv/rocksdb_engine.rs +++ b/src/storage/kv/rocksdb_engine.rs @@ -320,7 +320,7 @@ impl Snapshot for RocksSnapshot { mode: ScanMode, ) -> Result> { trace!("RocksSnapshot: create cf iterator"); - let iter = self.c().iterator_cf_opt(iter_opt, cf)?; + let iter = self.c().iterator_cf_opt(cf, iter_opt)?; Ok(Cursor::new(iter, mode)) } } From 5c3e33c10a097c3f95e70b2d4a47682c8b4b85b8 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 16:54:45 -0500 Subject: [PATCH 29/41] engine_traits: Rename some Peekable methods for consistency Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine.rs | 10 +++++----- components/engine_rocks/src/import.rs | 2 +- components/engine_rocks/src/snapshot.rs | 4 ++-- components/engine_traits/src/peekable.rs | 16 ++++++++-------- components/test_sst_importer/src/lib.rs | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index aaf4672d36b..1f801abe42e 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -105,13 +105,13 @@ impl Iterable for RocksEngine { } impl Peekable for RocksEngine { - fn get_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { let opt: RocksReadOptions = opts.into(); let v = self.0.get_opt(key, &opt.into_raw())?; Ok(v.map(|v| v.to_vec())) } - fn get_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { let opt: RocksReadOptions = opts.into(); let handle = get_cf_handle(&self.0, cf)?; let v = self.0.get_cf_opt(handle, key, &opt.into_raw())?; @@ -197,9 +197,9 @@ mod tests { engine.put(b"k1", b"v1").unwrap(); engine.put_cf(cf, b"k1", b"v2").unwrap(); - assert_eq!(&*engine.get(b"k1").unwrap().unwrap(), b"v1"); - assert!(engine.get_cf("foo", b"k1").is_err()); - assert_eq!(&*engine.get_cf(cf, b"k1").unwrap().unwrap(), b"v2"); + assert_eq!(&*engine.get_value(b"k1").unwrap().unwrap(), b"v1"); + assert!(engine.get_value_cf("foo", b"k1").is_err()); + assert_eq!(&*engine.get_value_cf(cf, b"k1").unwrap().unwrap(), b"v2"); } #[test] diff --git a/components/engine_rocks/src/import.rs b/components/engine_rocks/src/import.rs index 95206fb2e8e..bac884578fe 100644 --- a/components/engine_rocks/src/import.rs +++ b/components/engine_rocks/src/import.rs @@ -188,7 +188,7 @@ mod tests { fn check_db_with_kvs(db: &RocksEngine, cf: &str, kvs: &[(&str, &str)]) { for &(k, v) in kvs { - assert_eq!(db.get_cf(cf, k.as_bytes()).unwrap().unwrap(), v.as_bytes()); + assert_eq!(db.get_value_cf(cf, k.as_bytes()).unwrap().unwrap(), v.as_bytes()); } } diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index 29b7cde550f..bd692f9d6c5 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -100,7 +100,7 @@ impl Iterable for RocksSnapshot { } impl Peekable for RocksSnapshot { - fn get_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { @@ -110,7 +110,7 @@ impl Peekable for RocksSnapshot { Ok(v.map(|v| v.to_vec())) } - fn get_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { diff --git a/components/engine_traits/src/peekable.rs b/components/engine_traits/src/peekable.rs index 4fe3d6a2cef..626cb838168 100644 --- a/components/engine_traits/src/peekable.rs +++ b/components/engine_traits/src/peekable.rs @@ -3,19 +3,19 @@ use crate::*; pub trait Peekable { - fn get_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>>; - fn get_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>>; + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>>; + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>>; - fn get(&self, key: &[u8]) -> Result>> { - self.get_opt(&ReadOptions::default(), key) + fn get_value(&self, key: &[u8]) -> Result>> { + self.get_value_opt(&ReadOptions::default(), key) } - fn get_cf(&self, cf: &str, key: &[u8]) -> Result>> { - self.get_cf_opt(&ReadOptions::default(), cf, key) + fn get_value_cf(&self, cf: &str, key: &[u8]) -> Result>> { + self.get_value_cf_opt(&ReadOptions::default(), cf, key) } fn get_msg(&self, key: &[u8]) -> Result> { - let value = self.get(key)?; + let value = self.get_value(key)?; if value.is_none() { return Ok(None); } @@ -30,7 +30,7 @@ pub trait Peekable { cf: &str, key: &[u8], ) -> Result> { - let value = self.get_cf(cf, key)?; + let value = self.get_value_cf(cf, key)?; if value.is_none() { return Ok(None); } diff --git a/components/test_sst_importer/src/lib.rs b/components/test_sst_importer/src/lib.rs index d078370afc7..f843897ec02 100644 --- a/components/test_sst_importer/src/lib.rs +++ b/components/test_sst_importer/src/lib.rs @@ -47,7 +47,7 @@ where { for i in range.0..range.1 { let k = keys::data_key(&[i]); - assert_eq!(db.get(&k).unwrap().unwrap(), &[i]); + assert_eq!(db.get_value(&k).unwrap().unwrap(), &[i]); } } From b7369ed36bfc92a81b9bca19ab77548eb53fed26 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 17:22:39 -0500 Subject: [PATCH 30/41] engine_rocks add Compat for SyncSnapshot Signed-off-by: Brian Anderson --- components/engine_rocks/src/compat.rs | 11 +++++++++++ components/engine_rocks/src/snapshot.rs | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/components/engine_rocks/src/compat.rs b/components/engine_rocks/src/compat.rs index 726b301d58c..3bc414879db 100644 --- a/components/engine_rocks/src/compat.rs +++ b/components/engine_rocks/src/compat.rs @@ -4,7 +4,9 @@ use crate::engine::RocksEngine; use engine::DB; use std::sync::Arc; use engine::Snapshot as RawSnapshot; +use engine::SyncSnapshot as RawSyncSnapshot; use crate::snapshot::RocksSnapshot; +use crate::snapshot::RocksSyncSnapshot; /// A trait to enter the world of engine traits from a raw `Arc` /// with as little syntax as possible. @@ -34,3 +36,12 @@ impl Compat for RawSnapshot { RocksSnapshot::from_ref(self) } } + +impl Compat for RawSyncSnapshot { + type Other = RocksSyncSnapshot; + + #[inline] + fn c(&self) -> &RocksSyncSnapshot { + RocksSyncSnapshot::from_ref(self) + } +} diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index bd692f9d6c5..c289a382c7c 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -8,6 +8,7 @@ use engine_traits::{self, IterOptions, Iterable, Peekable, ReadOptions, Result, use rocksdb::rocksdb_options::UnsafeSnap; use rocksdb::{DBIterator, DB}; use engine::rocks::Snapshot as RawSnapshot; +use engine::rocks::SyncSnapshot as RawSyncSnapshot; use crate::options::RocksReadOptions; use crate::util::get_cf_handle; @@ -123,6 +124,7 @@ impl Peekable for RocksSnapshot { } #[derive(Clone, Debug)] +#[repr(C)] // Guarantee same representation as in engine/rocks pub struct RocksSyncSnapshot(Arc); impl Deref for RocksSyncSnapshot { @@ -137,4 +139,8 @@ impl RocksSyncSnapshot { pub fn new(db: Arc) -> RocksSyncSnapshot { RocksSyncSnapshot(Arc::new(RocksSnapshot::new(db))) } + + pub fn from_ref(raw: &RawSyncSnapshot) -> &RocksSyncSnapshot { + unsafe { &*(raw as *const _ as *const _) } + } } From c8f7c5212f384ac0cf520eb8d602c3152580cd10 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 18:10:41 -0500 Subject: [PATCH 31/41] engine_traits: Add DBVector This will be an associated type on Peekable to avoid copying results of queries. Signed-off-by: Brian Anderson --- components/engine_rocks/src/db_vector.rs | 38 +++++++++++++++++++++++ components/engine_rocks/src/lib.rs | 2 ++ components/engine_traits/src/db_vector.rs | 15 +++++++++ components/engine_traits/src/lib.rs | 2 ++ 4 files changed, 57 insertions(+) create mode 100644 components/engine_rocks/src/db_vector.rs create mode 100644 components/engine_traits/src/db_vector.rs diff --git a/components/engine_rocks/src/db_vector.rs b/components/engine_rocks/src/db_vector.rs new file mode 100644 index 00000000000..3930b4cb23c --- /dev/null +++ b/components/engine_rocks/src/db_vector.rs @@ -0,0 +1,38 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +use std::fmt::{self, Debug, Formatter}; +use std::ops::Deref; +use engine_traits::DBVector; +use rocksdb::DBVector as RawDBVector; + +pub struct RocksDBVector(RawDBVector); + +impl RocksDBVector { + pub fn from_raw(raw: RawDBVector) -> RocksDBVector { + RocksDBVector(raw) + } +} + +impl DBVector for RocksDBVector { +} + +impl Deref for RocksDBVector { + type Target = [u8]; + + fn deref(&self) -> &[u8] { + &self.0 + } +} + +impl Debug for RocksDBVector { + fn fmt(&self, formatter: &mut Formatter) -> fmt::Result { + write!(formatter, "{:?}", &**self) + } +} + + +impl<'a> PartialEq<&'a [u8]> for RocksDBVector { + fn eq(&self, rhs: &&[u8]) -> bool { + **rhs == **self + } +} diff --git a/components/engine_rocks/src/lib.rs b/components/engine_rocks/src/lib.rs index 51962fd376e..839dde46068 100644 --- a/components/engine_rocks/src/lib.rs +++ b/components/engine_rocks/src/lib.rs @@ -25,6 +25,8 @@ mod cf_options; pub use crate::cf_options::*; mod db_options; pub use crate::db_options::*; +mod db_vector; +pub use crate::db_vector::*; mod engine; pub use crate::engine::*; mod import; diff --git a/components/engine_traits/src/db_vector.rs b/components/engine_traits/src/db_vector.rs new file mode 100644 index 00000000000..b40ee45a89c --- /dev/null +++ b/components/engine_traits/src/db_vector.rs @@ -0,0 +1,15 @@ +// Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. + +use std::fmt::Debug; +use std::ops::Deref; + +/// A type that holds buffers queried from the database. +/// +/// The database may optimize this type to be a view into +/// its own cache. +pub trait DBVector: + Debug + + Deref + + for <'a> PartialEq<&'a [u8]> +{ +} diff --git a/components/engine_traits/src/lib.rs b/components/engine_traits/src/lib.rs index 4cb98734876..e34783073a4 100644 --- a/components/engine_traits/src/lib.rs +++ b/components/engine_traits/src/lib.rs @@ -72,6 +72,8 @@ mod cf_options; pub use crate::cf_options::*; mod db_options; pub use crate::db_options::*; +mod db_vector; +pub use crate::db_vector::*; mod engine; pub use crate::engine::*; mod import; From b97931b2279171fb460687d971483ae9730d516d Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 18:35:48 -0500 Subject: [PATCH 32/41] engine_traits: Modify Peekable to use DBVector Signed-off-by: Brian Anderson --- components/engine_rocks/src/engine.rs | 11 +++++++---- components/engine_rocks/src/snapshot.rs | 11 +++++++---- components/engine_traits/src/peekable.rs | 10 ++++++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index 1f801abe42e..3f022484c80 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -12,6 +12,7 @@ use rocksdb::{DBIterator, Writable, DB}; use crate::options::{RocksReadOptions, RocksWriteOptions}; use crate::util::get_cf_handle; use crate::{RocksEngineIterator, RocksSnapshot}; +use crate::db_vector::RocksDBVector; #[derive(Clone, Debug)] #[repr(transparent)] @@ -105,17 +106,19 @@ impl Iterable for RocksEngine { } impl Peekable for RocksEngine { - fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { + type DBVector = RocksDBVector; + + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result> { let opt: RocksReadOptions = opts.into(); let v = self.0.get_opt(key, &opt.into_raw())?; - Ok(v.map(|v| v.to_vec())) + Ok(v.map(RocksDBVector::from_raw)) } - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result> { let opt: RocksReadOptions = opts.into(); let handle = get_cf_handle(&self.0, cf)?; let v = self.0.get_cf_opt(handle, key, &opt.into_raw())?; - Ok(v.map(|v| v.to_vec())) + Ok(v.map(RocksDBVector::from_raw)) } } diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index c289a382c7c..109ba4e5eff 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -13,6 +13,7 @@ use engine::rocks::SyncSnapshot as RawSyncSnapshot; use crate::options::RocksReadOptions; use crate::util::get_cf_handle; use crate::RocksEngineIterator; +use crate::db_vector::RocksDBVector; #[repr(C)] // Guarantee same representation as in engine/rocks pub struct RocksSnapshot { @@ -101,17 +102,19 @@ impl Iterable for RocksSnapshot { } impl Peekable for RocksSnapshot { - fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>> { + type DBVector = RocksDBVector; + + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { opt.set_snapshot(&self.snap); } let v = self.db.get_opt(key, &opt)?; - Ok(v.map(|v| v.to_vec())) + Ok(v.map(RocksDBVector::from_raw)) } - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>> { + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { @@ -119,7 +122,7 @@ impl Peekable for RocksSnapshot { } let handle = get_cf_handle(self.db.as_ref(), cf)?; let v = self.db.get_cf_opt(handle, key, &opt)?; - Ok(v.map(|v| v.to_vec())) + Ok(v.map(RocksDBVector::from_raw)) } } diff --git a/components/engine_traits/src/peekable.rs b/components/engine_traits/src/peekable.rs index 626cb838168..c9c8794d9a3 100644 --- a/components/engine_traits/src/peekable.rs +++ b/components/engine_traits/src/peekable.rs @@ -3,14 +3,16 @@ use crate::*; pub trait Peekable { - fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>>; - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>>; + type DBVector: DBVector; - fn get_value(&self, key: &[u8]) -> Result>> { + fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>; + fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>; + + fn get_value(&self, key: &[u8]) -> Result> { self.get_value_opt(&ReadOptions::default(), key) } - fn get_value_cf(&self, cf: &str, key: &[u8]) -> Result>> { + fn get_value_cf(&self, cf: &str, key: &[u8]) -> Result> { self.get_value_cf_opt(&ReadOptions::default(), cf, key) } From 34acfffdc01e3f45f200529fcdaaa247c8c5825f Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 18:55:37 -0500 Subject: [PATCH 33/41] tikv: Generalize IntoProtoError to IntoOther Signed-off-by: Brian Anderson --- .../kv/into_protoerror.rs => into_other.rs} | 16 ++++++++++++---- src/lib.rs | 1 + src/storage/kv/mod.rs | 5 ++--- 3 files changed, 15 insertions(+), 7 deletions(-) rename src/{storage/kv/into_protoerror.rs => into_other.rs} (63%) diff --git a/src/storage/kv/into_protoerror.rs b/src/into_other.rs similarity index 63% rename from src/storage/kv/into_protoerror.rs rename to src/into_other.rs index 03d15dfeddd..708a969d1a6 100644 --- a/src/storage/kv/into_protoerror.rs +++ b/src/into_other.rs @@ -1,14 +1,22 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. +// Various conversions between types that can't have their own +// interdependencies. These are used to convert between error_traits::Error and +// other Error's that error_traits can't depend on. + use kvproto::errorpb::Error as ProtoError; use engine_traits::Error as EngineTraitsError; -pub trait IntoProtoError { - fn into_protoerror(self) -> ProtoError; +pub trait IntoOther { + type Other; + + fn into_other(self) -> Self::Other; } -impl IntoProtoError for EngineTraitsError { - fn into_protoerror(self) -> ProtoError { +impl IntoOther for EngineTraitsError { + type Other = ProtoError; + + fn into_other(self) -> ProtoError { let mut errorpb = ProtoError::default(); errorpb.set_message(format!("{}", self)); diff --git a/src/lib.rs b/src/lib.rs index ad238cdf20a..14eec54e62c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,6 +56,7 @@ extern crate failure; extern crate test; pub mod config; +pub mod into_other; pub mod coprocessor; pub mod import; pub mod raftstore; diff --git a/src/storage/kv/mod.rs b/src/storage/kv/mod.rs index c86228f8a05..03fea80490c 100644 --- a/src/storage/kv/mod.rs +++ b/src/storage/kv/mod.rs @@ -12,7 +12,7 @@ use kvproto::kvrpcpb::Context; use crate::raftstore::coprocessor::SeekRegionCallback; use crate::storage::{Key, Value}; -use self::into_protoerror::IntoProtoError; +use crate::into_other::IntoOther; mod btree_engine; mod compact_listener; @@ -20,7 +20,6 @@ mod cursor; mod perf_context; mod rocksdb_engine; mod stats; -mod into_protoerror; pub use self::btree_engine::{BTreeEngine, BTreeEngineIterator, BTreeEngineSnapshot}; pub use self::compact_listener::{CompactedEvent, CompactionListener}; @@ -189,7 +188,7 @@ impl From for Error { impl From for Error { fn from(err: engine_traits::Error) -> Error { - Error::Request(err.into_protoerror()) + Error::Request(err.into_other()) } } From df2c45dcfd6320776b7c8c702e714eac6cf58ee6 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 13 Nov 2019 20:22:53 -0500 Subject: [PATCH 34/41] tikv: Define IntoOther for engine_traits::Error Signed-off-by: Brian Anderson --- src/into_other.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/into_other.rs b/src/into_other.rs index 708a969d1a6..89f7c399890 100644 --- a/src/into_other.rs +++ b/src/into_other.rs @@ -4,18 +4,15 @@ // interdependencies. These are used to convert between error_traits::Error and // other Error's that error_traits can't depend on. +use raft::Error as RaftError; use kvproto::errorpb::Error as ProtoError; use engine_traits::Error as EngineTraitsError; -pub trait IntoOther { - type Other; - - fn into_other(self) -> Self::Other; +pub trait IntoOther { + fn into_other(self) -> O; } -impl IntoOther for EngineTraitsError { - type Other = ProtoError; - +impl IntoOther for EngineTraitsError { fn into_other(self) -> ProtoError { let mut errorpb = ProtoError::default(); errorpb.set_message(format!("{}", self)); @@ -30,3 +27,9 @@ impl IntoOther for EngineTraitsError { errorpb } } + +impl IntoOther for EngineTraitsError { + fn into_other(self) -> RaftError { + RaftError::Store(raft::StorageError::Other(self.into())) + } +} From 5e7d3087ac6e97b5952ee60f4f3d6189c574c1e5 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 14 Nov 2019 15:44:02 -0500 Subject: [PATCH 35/41] engine_traits: Add more crate documentation Signed-off-by: Brian Anderson --- components/engine_traits/src/lib.rs | 132 +++++++++++++++++++++++++--- 1 file changed, 122 insertions(+), 10 deletions(-) diff --git a/components/engine_traits/src/lib.rs b/components/engine_traits/src/lib.rs index e34783073a4..16dbf84c704 100644 --- a/components/engine_traits/src/lib.rs +++ b/components/engine_traits/src/lib.rs @@ -2,12 +2,71 @@ //! A generic TiKV storage engine //! -//! This is a work-in-progress attempt to abstract all the features -//! needed by TiKV to persist its data. +//! This is a work-in-progress attempt to abstract all the features needed by +//! TiKV to persist its data. //! -//! This crate must not have any transitive dependencies on RocksDB. +//! This crate must not have any transitive dependencies on RocksDB. The RocksDB +//! implementation is in the `engine_rocks` crate. //! -//! # Notes +//! This documentation contains a description of the porting process, current +//! design decisions and design guidelines, and refactoring tips. +//! +//! +//! # The porting process +//! +//! These are some guidelines that seem to make the porting managable. As the +//! process continues new strategies are discovered and written here. This is a +//! big refactoring and will take many monthse. +//! +//! Refactoring is a craft, not a science, and figuring out how to overcome any +//! particular situation takes experience and intuation, but these principles +//! can help. +//! +//! A guiding principle is to do one thing at a time. In particular, don't +//! redesign while encapsulating. +//! +//! The port is happening in stages: +//! +//! 1) Migrating the `engine` crate +//! 2) Eliminating the `rocksdb` dep from TiKV +//! 3) "Pulling up" the generic abstractions though TiKV +//! +//! These stages are described in more detail: +//! +//! ## 1) Migrating the `engine` crate +//! +//! Migrating the `engine` crate. The engine crate was an earlier attempt to +//! abstract the storage engine. Much of its structure is duplicated +//! near-identically in engine_traits, the difference being that engine_traits +//! has no RocksDB dependencies. Having no RocksDB dependencies makes it trivial +//! to guarantee that the abstractions are truly abstract. +//! +//! During this stage, we will eliminate the `engine` trait to reduce code +//! duplication. We do this by identifying a small subsystem within `engine`, +//! duplicating it within `engine_traits` and `engine_rocks`, deleting the code +//! from `engine`, and fixing all the callers to work with the abstracted +//! implementation. +//! +//! At the end of this stage the `engine` dependency will be deleted, but +//! TiKV will still depend on the concrete RocksDB implementations from +//! `engine_rocks`, as well as the raw API's from the `rocksdb` crate. +//! +//! ## 2) Eliminating the `rocksdb` dep from TiKV +//! +//! TiKV uses RocksDB via both the `rocksdb` crate and the `engine` crate. +//! During this stage we need to convert all callers to use the `engine_rocks` +//! crate instead. +//! +//! ## 3) "Pulling up" the generic abstractions through TiKv +//! +//! Finally, with all of TiKV using the `engine_traits` traits in conjunction +//! with the concrete `engine_rocks` types, we can push generic type parameters +//! up through the application. Then we will remove the concrete `engine_rocks` +//! dependency from TiKV so that it is impossible to re-introduce +//! engine-specific code again. +//! +//! +//! # Design notes //! //! - `KvEngine` is the main engine trait. It requires many other traits, which //! have many other associated types that implement yet more traits. @@ -17,13 +76,39 @@ //! a trait, and an "extension" trait that associates that type with `KvEngine`, //! which is part of `KvEngine's trait requirements. //! -//! - For now, for simplicity, all extension traits are required +//! - For now, for simplicity, all extension traits are required by `KvEngine`. +//! In the future it may be feasible to separate them for engines with +//! different feature sets. //! //! - Associated types generally have the same name as the trait they -//! are required to implement. +//! are required to implement. Engine extensions generally have the same +//! name suffixed with `Ext`. Concrete implementations usually have the +//! same name prefixed with the database name, i.e. `Rocks`. +//! +//! Example: +//! +//! ``` +//! // in engine_traits +//! +//! trait IOLimiterExt { +//! type IOLimiter: IOLimiter; +//! } +//! +//! trait IOLimiter { } +//! ``` +//! +//! ``` +//! // in engine_rust +//! +//! impl IOLimiterExt for RocksEngine { +//! type IOLimiter = RocksIOLimiter; +//! } +//! +//! impl IOLimiter for RocksIOLimiter { } +//! ``` //! //! - All engines use the same error type, defined in this crate. Thus -//! engine-specefic type information is boxed and hidden. +//! engine-specific type information is boxed and hidden. //! //! - `KvEngine` is a factory type for some of its associated types, but not //! others. For now, use factory methods when RocksDB would require factory @@ -32,13 +117,30 @@ //! use a standard new method). If future engines require factory methods, the //! traits can be converted then. //! -//! # Porting suggestions +//! - Types that require a handle to the engine (or some other "parent" type) +//! do so with either Rc or Arc. An example is EngineIterator. The reason +//! for this is that associated types cannot contain lifetimes. That requires +//! "generic associated types". See +//! +//! - https://github.com/rust-lang/rfcs/pull/1598 +//! - https://github.com/rust-lang/rust/issues/44265 +//! +//! +//! # Refactoring tips //! //! - Port modules with the fewest RocksDB dependencies at a time, modifying //! those modules's callers to convert to and from the engine traits as //! needed. Move in and out of the engine_traits world with the //! `RocksDB::from_ref` and `RocksDB::as_inner` methods. //! +//! - Down follow the type system too far "down the rabbit hole". When you see +//! that another subsystem is blocking you from refactoring the system you +//! are trying to refactor, stop, stash your changes, and focus on the other +//! system instead. +//! +//! - You will through away branches that lead to dead ends. Learn from the +//! experience and try again from a different angle. +//! //! - For now, use the same APIs as the RocksDB bindings, as methods //! on the various engine traits, and with this crate's error type. //! @@ -50,8 +152,18 @@ //! it in engine_traits and engine_rocks, replacing all the callers with calls //! into the traits, then delete the versions in the `engine` crate. //! -//! - Use the .c() method from engine_rocks::compat::EngineCompat to get a -//! KvEngine reference from Arc in the fewest characters. +//! - Use the .c() method from engine_rocks::compat::Compat to get a +//! KvEngine reference from Arc in the fewest characters. It also +//! works on Snapshot, and can be adapted to other types. +//! +//! - Use tikv::into_other::IntoOther to adapt between error types of dependencies +//! that are not themselves interdependent. E.g. raft::Error can be created +//! from engine_traits::Error even though neither `raft` tor `engine_traits` +//! know about each other. +//! +//! - "Plain old data" types in `engine` can be moved directly into +//! `engine_traits` and reexported from `engine` to ease the transition. +//! Likewise `engine_rocks` can temporarily call code from inside `engine`. #![recursion_limit = "200"] From f616eebee35a68a1b1df14c145ab0209395b3f65 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 14 Nov 2019 16:03:55 -0500 Subject: [PATCH 36/41] *: rustfmt Signed-off-by: Brian Anderson --- cmd/src/server.rs | 2 +- components/engine/src/iterable.rs | 2 +- components/engine/src/rocks/db.rs | 2 +- components/engine/src/rocks/snapshot.rs | 2 +- components/engine_rocks/src/compat.rs | 8 ++++---- components/engine_rocks/src/db_vector.rs | 8 +++----- components/engine_rocks/src/engine.rs | 9 +++++++-- components/engine_rocks/src/import.rs | 5 ++++- components/engine_rocks/src/options.rs | 2 +- components/engine_rocks/src/snapshot.rs | 13 +++++++++---- components/engine_traits/src/db_vector.rs | 7 +------ components/engine_traits/src/options.rs | 1 - components/engine_traits/src/peekable.rs | 7 ++++++- src/import/sst_service.rs | 9 ++++++--- src/into_other.rs | 6 +++--- src/lib.rs | 2 +- src/raftstore/store/region_snapshot.rs | 16 +++++++++------- src/server/debug.rs | 4 ++-- src/server/gc_worker.rs | 2 +- src/storage/kv/mod.rs | 2 +- src/storage/kv/rocksdb_engine.rs | 10 +++++----- 21 files changed, 67 insertions(+), 52 deletions(-) diff --git a/cmd/src/server.rs b/cmd/src/server.rs index bdee53e1c41..a4ed7439cf5 100644 --- a/cmd/src/server.rs +++ b/cmd/src/server.rs @@ -204,7 +204,7 @@ fn run_raft_server(pd_client: RpcClient, cfg: &TiKvConfig, security_mgr: Arc i64::max_value")); diff --git a/components/engine/src/iterable.rs b/components/engine/src/iterable.rs index e036a7665aa..16f79fde99b 100644 --- a/components/engine/src/iterable.rs +++ b/components/engine/src/iterable.rs @@ -5,8 +5,8 @@ pub use crate::rocks::{DBIterator, ReadOptions, DB}; use crate::Result; use tikv_util::keybuilder::KeyBuilder; -pub use engine_traits::SeekMode; pub use engine_traits::IterOptions as IterOption; +pub use engine_traits::SeekMode; pub trait IterOptionsExt { fn build_read_opts(self) -> ReadOptions; diff --git a/components/engine/src/rocks/db.rs b/components/engine/src/rocks/db.rs index 0851dc6934a..6fb53659579 100644 --- a/components/engine/src/rocks/db.rs +++ b/components/engine/src/rocks/db.rs @@ -3,8 +3,8 @@ use std::option::Option; use super::{util, DBIterator, DBVector, WriteBatch, DB}; -use crate::{IterOption, Iterable, Mutable, Peekable, Result}; use crate::iterable::IterOptionsExt; +use crate::{IterOption, Iterable, Mutable, Peekable, Result}; impl Peekable for DB { fn get_value(&self, key: &[u8]) -> Result> { diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index d853bb95046..3b1712a079c 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -6,8 +6,8 @@ use std::option::Option; use std::sync::Arc; use super::{CFHandle, DBVector, ReadOptions, UnsafeSnap, DB}; -use crate::{DBIterator, Error, IterOption, Iterable, Peekable, Result}; use crate::iterable::IterOptionsExt; +use crate::{DBIterator, Error, IterOption, Iterable, Peekable, Result}; #[repr(C)] // Guarantee same representation as in engine_rocks pub struct Snapshot { diff --git a/components/engine_rocks/src/compat.rs b/components/engine_rocks/src/compat.rs index 3bc414879db..5001327534b 100644 --- a/components/engine_rocks/src/compat.rs +++ b/components/engine_rocks/src/compat.rs @@ -1,12 +1,12 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. use crate::engine::RocksEngine; -use engine::DB; -use std::sync::Arc; -use engine::Snapshot as RawSnapshot; -use engine::SyncSnapshot as RawSyncSnapshot; use crate::snapshot::RocksSnapshot; use crate::snapshot::RocksSyncSnapshot; +use engine::Snapshot as RawSnapshot; +use engine::SyncSnapshot as RawSyncSnapshot; +use engine::DB; +use std::sync::Arc; /// A trait to enter the world of engine traits from a raw `Arc` /// with as little syntax as possible. diff --git a/components/engine_rocks/src/db_vector.rs b/components/engine_rocks/src/db_vector.rs index 3930b4cb23c..3a7fa4fa7ac 100644 --- a/components/engine_rocks/src/db_vector.rs +++ b/components/engine_rocks/src/db_vector.rs @@ -1,9 +1,9 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. -use std::fmt::{self, Debug, Formatter}; -use std::ops::Deref; use engine_traits::DBVector; use rocksdb::DBVector as RawDBVector; +use std::fmt::{self, Debug, Formatter}; +use std::ops::Deref; pub struct RocksDBVector(RawDBVector); @@ -13,8 +13,7 @@ impl RocksDBVector { } } -impl DBVector for RocksDBVector { -} +impl DBVector for RocksDBVector {} impl Deref for RocksDBVector { type Target = [u8]; @@ -30,7 +29,6 @@ impl Debug for RocksDBVector { } } - impl<'a> PartialEq<&'a [u8]> for RocksDBVector { fn eq(&self, rhs: &&[u8]) -> bool { **rhs == **self diff --git a/components/engine_rocks/src/engine.rs b/components/engine_rocks/src/engine.rs index 3f022484c80..896d200b485 100644 --- a/components/engine_rocks/src/engine.rs +++ b/components/engine_rocks/src/engine.rs @@ -9,10 +9,10 @@ use engine_traits::{ }; use rocksdb::{DBIterator, Writable, DB}; +use crate::db_vector::RocksDBVector; use crate::options::{RocksReadOptions, RocksWriteOptions}; use crate::util::get_cf_handle; use crate::{RocksEngineIterator, RocksSnapshot}; -use crate::db_vector::RocksDBVector; #[derive(Clone, Debug)] #[repr(transparent)] @@ -114,7 +114,12 @@ impl Peekable for RocksEngine { Ok(v.map(RocksDBVector::from_raw)) } - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result> { + fn get_value_cf_opt( + &self, + opts: &ReadOptions, + cf: &str, + key: &[u8], + ) -> Result> { let opt: RocksReadOptions = opts.into(); let handle = get_cf_handle(&self.0, cf)?; let v = self.0.get_cf_opt(handle, key, &opt.into_raw())?; diff --git a/components/engine_rocks/src/import.rs b/components/engine_rocks/src/import.rs index bac884578fe..e784a7259da 100644 --- a/components/engine_rocks/src/import.rs +++ b/components/engine_rocks/src/import.rs @@ -188,7 +188,10 @@ mod tests { fn check_db_with_kvs(db: &RocksEngine, cf: &str, kvs: &[(&str, &str)]) { for &(k, v) in kvs { - assert_eq!(db.get_value_cf(cf, k.as_bytes()).unwrap().unwrap(), v.as_bytes()); + assert_eq!( + db.get_value_cf(cf, k.as_bytes()).unwrap().unwrap(), + v.as_bytes() + ); } } diff --git a/components/engine_rocks/src/options.rs b/components/engine_rocks/src/options.rs index c13d4bc03c7..ffc6c8ce265 100644 --- a/components/engine_rocks/src/options.rs +++ b/components/engine_rocks/src/options.rs @@ -1,7 +1,7 @@ // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0. -use rocksdb::{ReadOptions as RawReadOptions, WriteOptions as RawWriteOptions}; use engine::IterOptionsExt; +use rocksdb::{ReadOptions as RawReadOptions, WriteOptions as RawWriteOptions}; pub struct RocksReadOptions(RawReadOptions); diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index 109ba4e5eff..ddd2ddce661 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -4,16 +4,16 @@ use std::fmt::{self, Debug, Formatter}; use std::ops::Deref; use std::sync::Arc; +use engine::rocks::Snapshot as RawSnapshot; +use engine::rocks::SyncSnapshot as RawSyncSnapshot; use engine_traits::{self, IterOptions, Iterable, Peekable, ReadOptions, Result, Snapshot}; use rocksdb::rocksdb_options::UnsafeSnap; use rocksdb::{DBIterator, DB}; -use engine::rocks::Snapshot as RawSnapshot; -use engine::rocks::SyncSnapshot as RawSyncSnapshot; +use crate::db_vector::RocksDBVector; use crate::options::RocksReadOptions; use crate::util::get_cf_handle; use crate::RocksEngineIterator; -use crate::db_vector::RocksDBVector; #[repr(C)] // Guarantee same representation as in engine/rocks pub struct RocksSnapshot { @@ -114,7 +114,12 @@ impl Peekable for RocksSnapshot { Ok(v.map(RocksDBVector::from_raw)) } - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result> { + fn get_value_cf_opt( + &self, + opts: &ReadOptions, + cf: &str, + key: &[u8], + ) -> Result> { let opt: RocksReadOptions = opts.into(); let mut opt = opt.into_raw(); unsafe { diff --git a/components/engine_traits/src/db_vector.rs b/components/engine_traits/src/db_vector.rs index b40ee45a89c..ec06dae4b7d 100644 --- a/components/engine_traits/src/db_vector.rs +++ b/components/engine_traits/src/db_vector.rs @@ -7,9 +7,4 @@ use std::ops::Deref; /// /// The database may optimize this type to be a view into /// its own cache. -pub trait DBVector: - Debug - + Deref - + for <'a> PartialEq<&'a [u8]> -{ -} +pub trait DBVector: Debug + Deref + for<'a> PartialEq<&'a [u8]> {} diff --git a/components/engine_traits/src/options.rs b/components/engine_traits/src/options.rs index 9f4e2e7ac62..ea72fba833c 100644 --- a/components/engine_traits/src/options.rs +++ b/components/engine_traits/src/options.rs @@ -177,4 +177,3 @@ impl Default for IterOptions { } } } - diff --git a/components/engine_traits/src/peekable.rs b/components/engine_traits/src/peekable.rs index c9c8794d9a3..ea2d378c9fd 100644 --- a/components/engine_traits/src/peekable.rs +++ b/components/engine_traits/src/peekable.rs @@ -6,7 +6,12 @@ pub trait Peekable { type DBVector: DBVector; fn get_value_opt(&self, opts: &ReadOptions, key: &[u8]) -> Result>; - fn get_value_cf_opt(&self, opts: &ReadOptions, cf: &str, key: &[u8]) -> Result>; + fn get_value_cf_opt( + &self, + opts: &ReadOptions, + cf: &str, + key: &[u8], + ) -> Result>; fn get_value(&self, key: &[u8]) -> Result> { self.get_value_opt(&ReadOptions::default(), key) diff --git a/src/import/sst_service.rs b/src/import/sst_service.rs index 6523ac3376d..74a48c438df 100644 --- a/src/import/sst_service.rs +++ b/src/import/sst_service.rs @@ -1,7 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. -use std::sync::{Arc, Mutex}; use std::convert::TryFrom; +use std::sync::{Arc, Mutex}; use engine::rocks::util::compact_files_in_range; use engine::rocks::DB; @@ -15,8 +15,8 @@ use kvproto::raft_cmdpb::*; use crate::raftstore::store::Callback; use crate::server::transport::RaftStoreRouter; use crate::server::CONFIG_ROCKSDB_GAUGE; -use engine_traits::IOLimiter; use engine_rocks::{RocksEngine, RocksIOLimiter}; +use engine_traits::IOLimiter; use sst_importer::send_rpc_response; use tikv_util::future::paired_future_callback; use tikv_util::time::Instant; @@ -299,7 +299,10 @@ impl ImportSst for ImportSSTService { let s = if let Ok(s) = s { s } else { - warn!("SetDownloadSpeedLimitRequest out of range: {}. Using i64::max_value", req.get_speed_limit()); + warn!( + "SetDownloadSpeedLimitRequest out of range: {}. Using i64::max_value", + req.get_speed_limit() + ); i64::max_value() }; diff --git a/src/into_other.rs b/src/into_other.rs index 89f7c399890..8359ec5443f 100644 --- a/src/into_other.rs +++ b/src/into_other.rs @@ -4,9 +4,9 @@ // interdependencies. These are used to convert between error_traits::Error and // other Error's that error_traits can't depend on. -use raft::Error as RaftError; -use kvproto::errorpb::Error as ProtoError; use engine_traits::Error as EngineTraitsError; +use kvproto::errorpb::Error as ProtoError; +use raft::Error as RaftError; pub trait IntoOther { fn into_other(self) -> O; @@ -25,7 +25,7 @@ impl IntoOther for EngineTraitsError { } errorpb - } + } } impl IntoOther for EngineTraitsError { diff --git a/src/lib.rs b/src/lib.rs index 14eec54e62c..f467c30a0a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,9 +56,9 @@ extern crate failure; extern crate test; pub mod config; -pub mod into_other; pub mod coprocessor; pub mod import; +pub mod into_other; pub mod raftstore; pub mod server; pub mod storage; diff --git a/src/raftstore/store/region_snapshot.rs b/src/raftstore/store/region_snapshot.rs index 72a906667f5..b1a5f0ad253 100644 --- a/src/raftstore/store/region_snapshot.rs +++ b/src/raftstore/store/region_snapshot.rs @@ -5,16 +5,16 @@ use engine::{ self, Error as EngineError, IterOption, Peekable, Result as EngineResult, Snapshot, SyncSnapshot, }; -use engine_traits::SeekKey; use engine_rocks::{Compat, RocksEngineIterator}; +use engine_traits::SeekKey; use kvproto::metapb::Region; use std::sync::Arc; use crate::raftstore::store::keys::DATA_PREFIX_KEY; use crate::raftstore::store::{keys, util, PeerStorage}; use crate::raftstore::Result; -use engine_traits::{Iterable, Iterator}; use engine_traits::util::check_key_in_range; +use engine_traits::{Iterable, Iterator}; use tikv_util::keybuilder::KeyBuilder; use tikv_util::metrics::CRITICAL_ERROR; use tikv_util::{panic_when_unexpected_key_or_data, set_panic_mark}; @@ -241,7 +241,9 @@ impl RegionIterator { update_upper_bound(&mut iter_opt, ®ion); let start_key = iter_opt.lower_bound().unwrap().to_vec(); let end_key = iter_opt.upper_bound().unwrap().to_vec(); - let iter = snap.c().iterator_opt(iter_opt) + let iter = snap + .c() + .iterator_opt(iter_opt) .expect("creating snapshot iterator"); // FIXME error handling RegionIterator { iter, @@ -262,7 +264,9 @@ impl RegionIterator { update_upper_bound(&mut iter_opt, ®ion); let start_key = iter_opt.lower_bound().unwrap().to_vec(); let end_key = iter_opt.upper_bound().unwrap().to_vec(); - let iter = snap.c().iterator_cf_opt(cf, iter_opt) + let iter = snap + .c() + .iterator_cf_opt(cf, iter_opt) .expect("creating snapshot iterator"); // FIXME error handling RegionIterator { iter, @@ -367,9 +371,7 @@ impl RegionIterator { #[inline] pub fn status(&self) -> Result<()> { - self.iter - .status() - .map_err(From::from) + self.iter.status().map_err(From::from) } #[inline] diff --git a/src/server/debug.rs b/src/server/debug.rs index ecc536ab0f3..c0a6794d945 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -12,9 +12,9 @@ use engine::rocks::{ CompactOptions, DBBottommostLevelCompaction, DBIterator as RocksIterator, Kv, ReadOptions, SeekKey, Writable, WriteBatch, WriteOptions, DB, }; +use engine::IterOptionsExt; use engine::{self, Engines, IterOption, Iterable, Mutable, Peekable}; use engine::{CF_DEFAULT, CF_LOCK, CF_RAFT, CF_WRITE}; -use engine::IterOptionsExt; use kvproto::debugpb::{self, Db as DBType, Module}; use kvproto::kvrpcpb::{MvccInfo, MvccLock, MvccValue, MvccWrite, Op}; use kvproto::metapb::{Peer, Region}; @@ -33,9 +33,9 @@ use crate::raftstore::store::{ write_peer_state, }; use crate::raftstore::store::{keys, PeerStorage}; +use crate::storage::kv::Iterator as EngineIterator; use crate::storage::mvcc::{Lock, LockType, Write, WriteType}; use crate::storage::types::Key; -use crate::storage::kv::Iterator as EngineIterator; use tikv_util::codec::bytes; use tikv_util::collections::HashSet; use tikv_util::config::ReadableSize; diff --git a/src/server/gc_worker.rs b/src/server/gc_worker.rs index b0f57686978..aa486d7ba62 100644 --- a/src/server/gc_worker.rs +++ b/src/server/gc_worker.rs @@ -1,7 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. -use std::convert::TryFrom; use std::cmp::Ordering; +use std::convert::TryFrom; use std::fmt::{self, Display, Formatter}; use std::mem; use std::sync::mpsc; diff --git a/src/storage/kv/mod.rs b/src/storage/kv/mod.rs index 03fea80490c..4924726b22a 100644 --- a/src/storage/kv/mod.rs +++ b/src/storage/kv/mod.rs @@ -10,9 +10,9 @@ use engine::{CfName, CF_DEFAULT}; use kvproto::errorpb::Error as ErrorHeader; use kvproto::kvrpcpb::Context; +use crate::into_other::IntoOther; use crate::raftstore::coprocessor::SeekRegionCallback; use crate::storage::{Key, Value}; -use crate::into_other::IntoOther; mod btree_engine; mod compact_listener; diff --git a/src/storage/kv/rocksdb_engine.rs b/src/storage/kv/rocksdb_engine.rs index af5496117ca..a3e18d5908f 100644 --- a/src/storage/kv/rocksdb_engine.rs +++ b/src/storage/kv/rocksdb_engine.rs @@ -9,13 +9,15 @@ use std::time::Duration; use engine::rocks; use engine::rocks::util::CFOptions; -use engine::rocks::{ColumnFamilyOptions, DBIterator, SeekKey as DBSeekKey, Writable, WriteBatch, DB}; +use engine::rocks::{ + ColumnFamilyOptions, DBIterator, SeekKey as DBSeekKey, Writable, WriteBatch, DB, +}; use engine::Engines; use engine::Error as EngineError; use engine::{CfName, CF_DEFAULT, CF_LOCK, CF_RAFT, CF_WRITE}; use engine::{IterOption, Peekable}; -use engine_traits::{Iterable, Iterator, SeekKey}; use engine_rocks::{Compat, RocksEngineIterator}; +use engine_traits::{Iterable, Iterator, SeekKey}; use kvproto::kvrpcpb::Context; use tempfile::{Builder, TempDir}; @@ -358,8 +360,7 @@ impl EngineIterator for RocksEngineIterator { } fn status(&self) -> Result<()> { - Iterator::status(self) - .map_err(From::from) + Iterator::status(self).map_err(From::from) } fn key(&self) -> &[u8] { @@ -418,7 +419,6 @@ impl + Send> EngineIterator for DBIterator { } } - #[cfg(test)] mod tests { pub use super::super::perf_context::{PerfStatisticsDelta, PerfStatisticsInstant}; From 148c559b5157d4606a11d5ef516352818d9d987b Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 19 Nov 2019 16:03:22 +0000 Subject: [PATCH 37/41] *: rustfmt Signed-off-by: Brian Anderson --- cmd/src/server.rs | 2 +- src/import/sst_service.rs | 9 ++++++--- src/server/debug.rs | 5 ++++- src/server/gc_worker.rs | 14 ++++++++------ 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/cmd/src/server.rs b/cmd/src/server.rs index cb1ca7f700f..32307975f03 100644 --- a/cmd/src/server.rs +++ b/cmd/src/server.rs @@ -204,7 +204,7 @@ fn run_raft_server(pd_client: RpcClient, cfg: &TiKvConfig, security_mgr: Arc i64::max_value")); diff --git a/src/import/sst_service.rs b/src/import/sst_service.rs index 6523ac3376d..74a48c438df 100644 --- a/src/import/sst_service.rs +++ b/src/import/sst_service.rs @@ -1,7 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. -use std::sync::{Arc, Mutex}; use std::convert::TryFrom; +use std::sync::{Arc, Mutex}; use engine::rocks::util::compact_files_in_range; use engine::rocks::DB; @@ -15,8 +15,8 @@ use kvproto::raft_cmdpb::*; use crate::raftstore::store::Callback; use crate::server::transport::RaftStoreRouter; use crate::server::CONFIG_ROCKSDB_GAUGE; -use engine_traits::IOLimiter; use engine_rocks::{RocksEngine, RocksIOLimiter}; +use engine_traits::IOLimiter; use sst_importer::send_rpc_response; use tikv_util::future::paired_future_callback; use tikv_util::time::Instant; @@ -299,7 +299,10 @@ impl ImportSst for ImportSSTService { let s = if let Ok(s) = s { s } else { - warn!("SetDownloadSpeedLimitRequest out of range: {}. Using i64::max_value", req.get_speed_limit()); + warn!( + "SetDownloadSpeedLimitRequest out of range: {}. Using i64::max_value", + req.get_speed_limit() + ); i64::max_value() }; diff --git a/src/server/debug.rs b/src/server/debug.rs index da61a8a730f..a598ff78e03 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -1,6 +1,7 @@ // Copyright 2017 TiKV Project Authors. Licensed under Apache-2.0. use std::cmp::Ordering; +use std::convert::TryFrom; use std::iter::FromIterator; use std::str::FromStr; use std::sync::Arc; @@ -809,11 +810,13 @@ impl Debugger { Module::Server => { if config_name == GC_IO_LIMITER_CONFIG_NAME { if let Ok(bytes_per_sec) = ReadableSize::from_str(config_value) { + let bps = i64::try_from(bytes_per_sec.0) + .expect(&format!("{} > i64::max_value", GC_IO_LIMITER_CONFIG_NAME)); return self .gc_worker .as_ref() .expect("must be some") - .change_io_limit(bytes_per_sec.0) + .change_io_limit(bps) .map_err(|e| Error::Other(e.into())); } } diff --git a/src/server/gc_worker.rs b/src/server/gc_worker.rs index abb5c68f00d..1e330abf19d 100644 --- a/src/server/gc_worker.rs +++ b/src/server/gc_worker.rs @@ -1,7 +1,7 @@ // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0. -use std::convert::TryFrom; use std::cmp::Ordering; +use std::convert::TryFrom; use std::fmt::{self, Display, Formatter}; use std::mem; use std::sync::mpsc; @@ -180,7 +180,7 @@ impl GCRunner { engine: E, local_storage: Option>, raft_store_router: Option, - limiter: Arc>>, + limiter: Arc>>, cfg: GCConfig, ) -> Self { Self { @@ -1100,7 +1100,7 @@ pub struct GCWorker { raft_store_router: Option, cfg: Option, - limiter: Arc>>, + limiter: Arc>>, /// How many strong references. The worker will be stopped /// once there are no more references. @@ -1160,7 +1160,9 @@ impl GCWorker { )); let worker_scheduler = worker.lock().unwrap().scheduler(); let limiter = if cfg.max_write_bytes_per_sec.0 > 0 { - Some(IOLimiter::new(cfg.max_write_bytes_per_sec.0)) + let bps = i64::try_from(cfg.max_write_bytes_per_sec.0) + .expect("snap_max_write_bytes_per_sec > i64::max_value"); + Some(IOLimiter::new(bps)) } else { None }; @@ -1251,13 +1253,13 @@ impl GCWorker { .or_else(handle_gc_task_schedule_error) } - pub fn change_io_limit(&self, limit: u64) -> Result<()> { + pub fn change_io_limit(&self, limit: i64) -> Result<()> { let mut limiter = self.limiter.lock().unwrap(); if limit == 0 { limiter.take(); } else { limiter - .get_or_insert_with(|| IOLimiter::new(limit)) + .get_or_insert_with(|| RocksIOLimiter::new(limit)) .set_bytes_per_second(limit as i64); } info!("GC io limit changed"; "max_write_bytes_per_sec" => limit); From ae0bc5d96a30ac508aa6da9985b48046b1d34c68 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 19 Nov 2019 16:20:30 +0000 Subject: [PATCH 38/41] tikv: clippy Signed-off-by: Brian Anderson --- src/server/debug.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/debug.rs b/src/server/debug.rs index 97a65d92792..f399b550996 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -812,7 +812,7 @@ impl Debugger { if config_name == GC_IO_LIMITER_CONFIG_NAME { if let Ok(bytes_per_sec) = ReadableSize::from_str(config_value) { let bps = i64::try_from(bytes_per_sec.0) - .expect(&format!("{} > i64::max_value", GC_IO_LIMITER_CONFIG_NAME)); + .unwrap_or_else(|_| (panic!("{} > i64::max_value", GC_IO_LIMITER_CONFIG_NAME))); return self .gc_worker .as_ref() From ce7005edd12c9c11fc39197d49099c757a9dfca9 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 19 Nov 2019 16:31:52 +0000 Subject: [PATCH 39/41] engine_traits: Change titan_key_only to key_only Signed-off-by: Brian Anderson --- components/engine/src/iterable.rs | 2 +- components/engine/src/util.rs | 2 +- components/engine_traits/src/options.rs | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/components/engine/src/iterable.rs b/components/engine/src/iterable.rs index 16f79fde99b..885eda33029 100644 --- a/components/engine/src/iterable.rs +++ b/components/engine/src/iterable.rs @@ -16,7 +16,7 @@ impl IterOptionsExt for IterOption { fn build_read_opts(self) -> ReadOptions { let mut opts = ReadOptions::new(); opts.fill_cache(self.fill_cache()); - if self.titan_key_only() { + if self.key_only() { opts.set_titan_key_only(true); } if self.total_order_seek_used() { diff --git a/components/engine/src/util.rs b/components/engine/src/util.rs index 4cf7e57ff18..47e512e1b50 100644 --- a/components/engine/src/util.rs +++ b/components/engine/src/util.rs @@ -47,7 +47,7 @@ pub fn delete_all_in_range_cf( if db.is_titan() { // Cause DeleteFilesInRange may expose old blob index keys, setting key only for Titan // to avoid referring to missing blob files. - iter_opt.set_titan_key_only(true); + iter_opt.set_key_only(true); } let mut it = db.new_iterator_cf(cf, iter_opt)?; it.seek(start_key.into()); diff --git a/components/engine_traits/src/options.rs b/components/engine_traits/src/options.rs index ea72fba833c..76ebdae6e68 100644 --- a/components/engine_traits/src/options.rs +++ b/components/engine_traits/src/options.rs @@ -53,7 +53,7 @@ pub struct IterOptions { prefix_same_as_start: bool, fill_cache: bool, // only supported when Titan enabled, otherwise it doesn't take effect. - titan_key_only: bool, + key_only: bool, seek_mode: SeekMode, } @@ -68,7 +68,7 @@ impl IterOptions { upper_bound, prefix_same_as_start: false, fill_cache, - titan_key_only: false, + key_only: false, seek_mode: SeekMode::TotalOrder, } } @@ -95,13 +95,13 @@ impl IterOptions { } #[inline] - pub fn titan_key_only(&self) -> bool { - self.titan_key_only + pub fn key_only(&self) -> bool { + self.key_only } #[inline] - pub fn set_titan_key_only(&mut self, v: bool) { - self.titan_key_only = v; + pub fn set_key_only(&mut self, v: bool) { + self.key_only = v; } #[inline] @@ -172,7 +172,7 @@ impl Default for IterOptions { upper_bound: None, prefix_same_as_start: false, fill_cache: true, - titan_key_only: false, + key_only: false, seek_mode: SeekMode::TotalOrder, } } From 76dff856960f4a5de1718c7fcb40368e5f0970ff Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Tue, 19 Nov 2019 16:41:08 +0000 Subject: [PATCH 40/41] tikv: rustfmt Signed-off-by: Brian Anderson --- src/server/debug.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/server/debug.rs b/src/server/debug.rs index f399b550996..c7d397c538f 100644 --- a/src/server/debug.rs +++ b/src/server/debug.rs @@ -811,8 +811,9 @@ impl Debugger { Module::Server => { if config_name == GC_IO_LIMITER_CONFIG_NAME { if let Ok(bytes_per_sec) = ReadableSize::from_str(config_value) { - let bps = i64::try_from(bytes_per_sec.0) - .unwrap_or_else(|_| (panic!("{} > i64::max_value", GC_IO_LIMITER_CONFIG_NAME))); + let bps = i64::try_from(bytes_per_sec.0).unwrap_or_else(|_| { + (panic!("{} > i64::max_value", GC_IO_LIMITER_CONFIG_NAME)) + }); return self .gc_worker .as_ref() From 14e8e5dd9ef3096b6313a6f573922203b3e359ce Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Thu, 21 Nov 2019 19:11:15 +0000 Subject: [PATCH 41/41] engine_rocks: Fix repr of SyncSnapshot Signed-off-by: Brian Anderson --- components/engine/src/rocks/snapshot.rs | 1 + components/engine_rocks/src/snapshot.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/components/engine/src/rocks/snapshot.rs b/components/engine/src/rocks/snapshot.rs index 3b1712a079c..e525cd0c204 100644 --- a/components/engine/src/rocks/snapshot.rs +++ b/components/engine/src/rocks/snapshot.rs @@ -62,6 +62,7 @@ impl Drop for Snapshot { } #[derive(Debug, Clone)] +#[repr(transparent)] // Guarantee same representation as in engine_rocks pub struct SyncSnapshot(Arc); impl Deref for SyncSnapshot { diff --git a/components/engine_rocks/src/snapshot.rs b/components/engine_rocks/src/snapshot.rs index ddd2ddce661..da1e8fc4fc6 100644 --- a/components/engine_rocks/src/snapshot.rs +++ b/components/engine_rocks/src/snapshot.rs @@ -132,7 +132,7 @@ impl Peekable for RocksSnapshot { } #[derive(Clone, Debug)] -#[repr(C)] // Guarantee same representation as in engine/rocks +#[repr(transparent)] // Guarantee same representation as in engine/rocks pub struct RocksSyncSnapshot(Arc); impl Deref for RocksSyncSnapshot {