*: Support both Prost and rust-protobuf libraries

To use Prost, set the `PROST` env var, e.g.,: `PROST=1 make dev`. If using Cargo, use `--no-default-features --features prost-codec`. The most notable change is threading the prost-codec/protobuf-codec through the Cargo.tomls of all crates. In addition, in order to make this work I had to move integraton tests and benchmarks into their own crate (`tests`). This is because Cargo features do not interact perfectly with dev-dependencies. We're using a Git dep for Prost in order to get some optimisations which are on master, but not in the latest release. We can change to a crates.io dep when there is another release. We must allow the `identity_conversion` lint because there are some conversions which are meaningful with rust-protobuf, but no-ops with Prost. The changes to src/coprocessor/endpoint.rs are because Prost does not permit setting a custom recursion limit. We only did this for tests previously. We now use the default recursion limit all the time for both codecs; the test must be adjusted so that we hit the higher limit. Signed-off-by: Nick Cameron <nrc@ncameron.org>
tikv · Sep 10, 2019 · b294b71 · b294b71
1 parent ff82aa9
commit b294b71
Show file tree

Hide file tree

Showing 94 changed files with 678 additions and 241 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -12,52 +12,46 @@ edition = "2018"
 publish = false
 
 [features]
-default = ["jemalloc"]
+default = ["default_alloc", "protobuf-codec"]
+default_alloc = ["jemalloc"]
 tcmalloc = ["tikv_alloc/tcmalloc"]
 jemalloc = ["tikv_alloc/jemalloc", "engine/jemalloc"]
 mimalloc = ["tikv_alloc/mimalloc"]
 portable = ["engine/portable"]
 sse = ["engine/sse"]
 mem-profiling = ["tikv_alloc/mem-profiling"]
-profiling = ["profiler/profiling"]
 failpoints = ["fail/failpoints"]
+prost-codec = [
+  "tipb/prost-codec",
+  "kvproto/prost-codec",
+  "tikv_util/prost-codec",
+  "test_util/prost-codec",
+  "engine/prost-codec",
+  "grpcio/prost-codec",
+  "raft/prost-codec",
+  "tidb_query/prost-codec",
+  "log_wrappers/prost-codec",
+  "pd_client/prost-codec",
+  "keys/prost-codec",
+  "prost",
+]
+protobuf-codec = [
+  "tipb/protobuf-codec",
+  "kvproto/protobuf-codec",
+  "tikv_util/protobuf-codec",
+  "test_util/protobuf-codec",
+  "engine/protobuf-codec",
+  "grpcio/protobuf-codec",
+  "raft/protobuf-codec",
+  "tidb_query/protobuf-codec",
+  "log_wrappers/protobuf-codec",
+  "pd_client/protobuf-codec",
+  "keys/protobuf-codec",
+]
 
 [lib]
 name = "tikv"
 
-[[test]]
-name = "failpoints"
-path = "tests/failpoints/mod.rs"
-required-features = ["failpoints"]
-
-[[test]]
-name = "integrations"
-path = "tests/integrations/mod.rs"
-
-[[bench]]
-name = "raftstore"
-harness = false
-path = "benches/raftstore/mod.rs"
-
-[[bench]]
-name = "coprocessor_executors"
-harness = false
-path = "benches/coprocessor_executors/mod.rs"
-
-[[bench]]
-name = "hierarchy"
-harness = false
-path = "benches/hierarchy/mod.rs"
-
-[[bench]]
-name = "misc"
-path = "benches/misc/mod.rs"
-
-[[bench]]
-name = "deadlock_detector"
-harness = false
-path = "benches/deadlock_detector/mod.rs"
-
 [dependencies]
 log = { version = "0.3", features = ["max_level_trace", "release_max_level_debug"] }
 slog = { version = "2.3", features = ["max_level_trace", "release_max_level_debug"] }
@@ -75,6 +69,7 @@ libc = "0.2"
 crc = "1.8"
 fs2 = "0.4"
 spin = "0.5"
+prost = { git = "https://github.com/danburkert/prost", rev = "1944c27c3029d01ff216e7b126d846b6cf8c7d77", optional = true }
 protobuf = "2.8"
 nix = "0.11"
 utime = "0.2"
@@ -96,8 +91,8 @@ zipf = "5.0.1"
 bitflags = "1.0.1"
 fail = "0.3"
 uuid = { version = "0.6", features = [ "serde", "v4" ] }
-grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ] }
-raft = "0.6.0-alpha"
+grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ], default-features = false }
+raft = { version = "0.6.0-alpha" , default-features = false }
 crossbeam = "0.5"
 derive_more = "0.15.0"
 hex = "0.3"
@@ -110,16 +105,17 @@ vlog = "0.1.4"
 mime = "0.3.13"
 farmhash = "1.1.5"
 failure = "0.1"
-tipb = { git = "https://github.com/pingcap/tipb.git" }
-kvproto = { git = "https://github.com/pingcap/kvproto.git" }
+tipb = { git = "https://github.com/pingcap/tipb.git", default-features = false }
+kvproto = { git = "https://github.com/pingcap/kvproto.git", default-features = false }
 
-tikv_alloc = { path = "components/tikv_alloc", default-features = false }
+tikv_alloc = { path = "components/tikv_alloc" }
 tikv_util = { path = "components/tikv_util" }
 log_wrappers = { path = "components/log_wrappers" }
 engine = { path = "components/engine" }
 tidb_query = { path = "components/tidb_query" }
 pd_client = { path = "components/pd_client" }
 keys = { path = "components/keys" }
+test_util = { path = "components/test_util" }
 
 [dependencies.murmur3]
 git = "https://github.com/pingcap/murmur3.git"
@@ -133,6 +129,9 @@ features = ["nightly", "push", "process"]
 version = "0.1.4"
 default-features = false
 
+[dev-dependencies]
+panic_hook = { path = "components/panic_hook" }
+
 [replace]
 # TODO: remove this when slog is compatible with the log 0.4
 "log:0.3.9" = { git = "https://github.com/busyjay/log", branch = "use-static-module" }
@@ -142,22 +141,7 @@ default-features = false
 "raft-proto:0.6.0-alpha" = { git = "https://github.com/pingcap/raft-rs", branch = "master" }
 "protobuf:2.8.0" = { git = "https://github.com/nrc/rust-protobuf", branch = "v2.8" }
 "protobuf-codegen:2.8.0" = { git = "https://github.com/nrc/rust-protobuf", branch = "v2.8" }
-
-[dev-dependencies]
-# See https://bheisler.github.io/criterion.rs/book/user_guide/known_limitations.html for the usage
-# of `real_blackbox` feature.
-criterion = { version = "0.2.11", features=['real_blackbox'] }
-arrow = "0.10.0"
-rand_xorshift = "0.1"
-
-profiler = { path = "components/profiler" }
-panic_hook = { path = "components/panic_hook" }
-tipb_helper = { path = "components/tipb_helper" }
-tidb_query_datatype = { path = "components/tidb_query_datatype" }
-test_util = { path = "components/test_util" }
-test_raftstore = { path = "components/test_raftstore" }
-test_storage = { path = "components/test_storage" }
-test_coprocessor = { path = "components/test_coprocessor" }
+"prost:0.5.0" = { git = "https://github.com/danburkert/prost", rev = "1944c27c3029d01ff216e7b126d846b6cf8c7d77" }
 
 [target.'cfg(unix)'.dependencies]
 signal = "0.6"
@@ -171,6 +155,7 @@ members = [
   "fuzz/fuzzer-afl",
   "fuzz/fuzzer-libfuzzer",
   "fuzz/fuzzer-honggfuzz",
+  "tests",
   "components/test_raftstore",
   "components/test_storage",
   "components/test_coprocessor",

diff --git a/Makefile b/Makefile
@@ -65,6 +65,13 @@ ifeq ($(FAIL_POINT),1)
 ENABLE_FEATURES += failpoints
 endif
 
+# Use Prost instead of rust-protobuf to encode and decode protocol buffers.
+ifeq ($(PROST),1)
+ENABLE_FEATURES += prost-codec
+else
+ENABLE_FEATURES += protobuf-codec
+endif
+
 PROJECT_DIR:=$(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))
 
 DEPS_PATH = $(CURDIR)/tmp
@@ -207,14 +214,15 @@ pre-clippy: unset-override
 	@rustup component add clippy
 
 clippy: pre-clippy
-	@cargo clippy --all --all-targets -- \
+	@cargo clippy --all --all-targets --no-default-features --features "${ENABLE_FEATURES}" -- \
 		-A clippy::module_inception -A clippy::needless_pass_by_value -A clippy::cognitive_complexity \
 		-A clippy::unreadable_literal -A clippy::should_implement_trait -A clippy::verbose_bit_mask \
 		-A clippy::implicit_hasher -A clippy::large_enum_variant -A clippy::new_without_default \
 		-A clippy::neg_cmp_op_on_partial_ord -A clippy::too_many_arguments \
 		-A clippy::excessive_precision -A clippy::collapsible_if -A clippy::blacklisted_name \
 		-A clippy::needless_range_loop -A clippy::redundant_closure \
-		-A clippy::match_wild_err_arm -A clippy::blacklisted_name -A clippy::redundant_closure_call
+		-A clippy::match_wild_err_arm -A clippy::blacklisted_name -A clippy::redundant_closure_call \
+		-A clippy::identity_conversion
 
 pre-audit:
 	$(eval LATEST_AUDIT_VERSION := $(strip $(shell cargo search cargo-audit | head -n 1 | awk '{ gsub(/"/, "", $$3); print $$3 }')))

diff --git a/ci-build/test.sh b/ci-build/test.sh
@@ -25,6 +25,7 @@ make clippy || panic "\e[35mplease fix the errors!!!\e[0m"
 
 if [[ "$SKIP_TESTS" != "true" ]]; then
     make test 2>&1 | tee tests.out
+    PROST=1 make test 2>&1 | tee tests-prost.out
 else
     EXTRA_CARGO_ARGS="--no-run" make test
     exit $?

diff --git a/cmd/Cargo.toml b/cmd/Cargo.toml
@@ -6,15 +6,32 @@ edition = "2018"
 publish = false
 
 [features]
-default = ["tikv/default"]
+default = ["tikv/default_alloc"]
 tcmalloc = ["tikv/tcmalloc"]
 jemalloc = ["tikv/jemalloc"]
 mimalloc = ["tikv/mimalloc"]
 portable = ["tikv/portable"]
 sse = ["tikv/sse"]
 mem-profiling = ["tikv/mem-profiling"]
-profiling = ["tikv/profiling"]
 failpoints = ["tikv/failpoints"]
+prost-codec = [
+  "raft/prost-codec",
+  "kvproto/prost-codec",
+  "tikv/prost-codec",
+  "grpcio/prost-codec",
+  "tikv_util/prost-codec",
+  "engine/prost-codec",
+  "pd_client/prost-codec",
+]
+protobuf-codec = [
+  "raft/protobuf-codec",
+  "kvproto/protobuf-codec",
+  "tikv/protobuf-codec",
+  "grpcio/protobuf-codec",
+  "tikv_util/protobuf-codec",
+  "engine/protobuf-codec",
+  "pd_client/protobuf-codec",
+]
 
 [lib]
 name = "cmd"
@@ -34,7 +51,7 @@ rand = "0.6.5"
 toml = "0.4"
 libc = "0.2"
 fs2 = "0.4"
-protobuf = "2"
+protobuf = "2.8"
 nix = "0.11"
 chrono = "0.4"
 clap = "2.32"
@@ -43,11 +60,11 @@ futures = "0.1"
 serde = "1.0"
 serde_json = "1.0"
 fail = "0.3"
-grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ] }
-raft = "0.6.0-alpha"
+grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ], default-features = false }
+raft = { version = "0.6.0-alpha" , default-features = false }
 hex = "0.3"
 vlog = "0.1.4"
-kvproto = { git = "https://github.com/pingcap/kvproto.git" }
+kvproto = { git = "https://github.com/pingcap/kvproto.git", default-features = false }
 tikv_util = { path = "../components/tikv_util" }
 engine = { path = "../components/engine" }
 pd_client = { path = "../components/pd_client" }

diff --git a/components/backup/Cargo.toml b/components/backup/Cargo.toml
@@ -4,20 +4,38 @@ version = "0.0.1"
 edition = "2018"
 publish = false
 
+[features]
+prost-codec = [
+  "tikv_util/prost-codec",
+  "tikv/prost-codec",
+  "kvproto/prost-codec",
+  "raft/prost-codec",
+  "grpcio/prost-codec",
+  "engine/prost-codec",
+]
+protobuf-codec = [
+  "tikv_util/protobuf-codec",
+  "tikv/protobuf-codec",
+  "kvproto/protobuf-codec",
+  "raft/protobuf-codec",
+  "grpcio/protobuf-codec",
+  "engine/protobuf-codec",
+]
+
 [dependencies]
 tikv = { path = "../../", default-features = false }
-kvproto = { git = "https://github.com/pingcap/kvproto.git" }
+kvproto = { git = "https://github.com/pingcap/kvproto.git", default-features = false }
 protobuf = "2.8"
-raft = "0.6.0-alpha"
+raft = { version = "0.6.0-alpha" , default-features = false }
 slog = { version = "2.3", features = ["max_level_trace", "release_max_level_debug"] }
 # better to not use slog-global, but pass in the logger
 slog-global = { version = "0.1", git = "https://github.com/breeswish/slog-global.git", rev = "91904ade" }
-engine = { path = "../engine" }
-tikv_util = { path = "../tikv_util" }
+engine = { path = "../engine", default-features = false }
+tikv_util = { path = "../tikv_util", default-features = false }
 futures = "0.1"
 tempfile = "3.0"
 tokio-threadpool = "0.1"
-grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ] }
+grpcio = { version = "0.5.0-alpha.3", features = [ "openssl-vendored" ], default-features = false }
 url = "2.0"
 hex = "0.3"
 external_storage = { path = "../external_storage" }
@@ -33,6 +51,6 @@ default-features = false
 features = ["nightly", "push", "process"]
 
 [dev-dependencies]
-test_util = { path = "../test_util" }
-test_raftstore = { path = "../test_raftstore" }
+test_util = { path = "../test_util", default-features = false }
+test_raftstore = { path = "../test_raftstore", default-features = false }
 uuid = { version = "0.6", features = [ "serde", "v4" ] }
diff --git a/components/backup/src/endpoint.rs b/components/backup/src/endpoint.rs
@@ -200,7 +200,7 @@ impl<E: Engine, R: RegionInfoProvider> Endpoint<E, R> {
         let _ = start_ts;
 
         let backup_ts = end_ts;
-        let mut ctx = Context::new();
+        let mut ctx = Context::default();
         ctx.set_region_id(brange.region.get_id());
         ctx.set_region_epoch(brange.region.get_region_epoch().to_owned());
         ctx.set_peer(brange.leader.clone());
@@ -308,7 +308,7 @@ impl<E: Engine, R: RegionInfoProvider> Endpoint<E, R> {
             let end_key = brange
                 .end_key
                 .map_or_else(|| vec![], |k| k.into_raw().unwrap());
-            let mut response = BackupResponse::new();
+            let mut response = BackupResponse::default();
             response.set_start_key(start_key.clone());
             response.set_end_key(end_key.clone());
             match res {
@@ -652,7 +652,7 @@ pub mod tests {
 
         // TODO: check key number for each snapshot.
         for (ts, len) in backup_tss {
-            let mut req = BackupRequest::new();
+            let mut req = BackupRequest::default();
             req.set_start_key(vec![]);
             req.set_end_key(vec![b'5']);
             req.set_start_version(ts);
@@ -708,7 +708,7 @@ pub mod tests {
         );
 
         let now = alloc_ts();
-        let mut req = BackupRequest::new();
+        let mut req = BackupRequest::default();
         req.set_start_key(vec![]);
         req.set_end_key(vec![b'5']);
         req.set_start_version(now);
@@ -784,7 +784,7 @@ pub mod tests {
         must_commit(&engine, key.as_bytes(), start, commit);
 
         let now = alloc_ts();
-        let mut req = BackupRequest::new();
+        let mut req = BackupRequest::default();
         req.set_start_key(vec![]);
         req.set_end_key(vec![]);
         req.set_start_version(now);

diff --git a/components/backup/src/errors.rs b/components/backup/src/errors.rs
@@ -13,7 +13,7 @@ use tikv::storage::txn::Error as TxnError;
 impl Into<ErrorPb> for Error {
     // TODO: test error conversion.
     fn into(self) -> ErrorPb {
-        let mut err = ErrorPb::new();
+        let mut err = ErrorPb::default();
         match self {
             Error::ClusterID { current, request } => {
                 err.mut_cluster_id_error().set_current(current);
@@ -25,7 +25,7 @@ impl Into<ErrorPb> for Error {
                 err.set_region_error(e);
             }
             Error::Txn(TxnError::Mvcc(MvccError::KeyIsLocked(info))) => {
-                let mut e = KeyError::new();
+                let mut e = KeyError::default();
                 e.set_locked(info);
                 err.set_kv_error(e);
             }