diff --git a/CHANGELOG.md b/CHANGELOG.md index c27405d0..bddbd4f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +## [0.4.0] - 2023-09-01 + ### Behavior Changes * `LogBatch::put` returns a `Result<()>` instead of `()`. It errs when the key is reserved for internal use. diff --git a/Cargo.toml b/Cargo.toml index 177ae280..b88e3e59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "raft-engine" -version = "0.3.0" +version = "0.4.0" authors = ["The TiKV Project Developers"] edition = "2018" rust-version = "1.66.0" diff --git a/Makefile b/Makefile index 4bab52f0..da01ab1b 100644 --- a/Makefile +++ b/Makefile @@ -41,12 +41,13 @@ clean: format: cargo ${TOOLCHAIN_ARGS} fmt --all +CLIPPY_WHITELIST += -A clippy::bool_assert_comparison ## Run clippy. clippy: ifdef WITH_NIGHTLY_FEATURES - cargo ${TOOLCHAIN_ARGS} clippy --all --features nightly_group,failpoints --all-targets -- -D clippy::all + cargo ${TOOLCHAIN_ARGS} clippy --all --features nightly_group,failpoints --all-targets -- -D clippy::all ${CLIPPY_WHITELIST} else - cargo ${TOOLCHAIN_ARGS} clippy --all --features failpoints --all-targets -- -D clippy::all + cargo ${TOOLCHAIN_ARGS} clippy --all --features failpoints --all-targets -- -D clippy::all ${CLIPPY_WHITELIST} endif ## Run tests. diff --git a/README.md b/README.md index f70a1b1b..0caff515 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ Put this in your Cargo.toml: ```rust [dependencies] -raft-engine = "0.3.0" +raft-engine = "0.4.0" ``` Available Cargo features: diff --git a/ctl/Cargo.toml b/ctl/Cargo.toml index c10b092e..54b7ee87 100644 --- a/ctl/Cargo.toml +++ b/ctl/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "raft-engine-ctl" -version = "0.3.0" +version = "0.4.0" authors = ["The TiKV Project Developers"] edition = "2018" rust-version = "1.61.0" @@ -11,4 +11,4 @@ license = "Apache-2.0" [dependencies] clap = { version = "3.1", features = ["derive", "cargo"] } env_logger = "0.10" -raft-engine = { path = "..", version = "0.3.0", features = ["scripting", "internals"] } +raft-engine = { path = "..", version = "0.4.0", features = ["scripting", "internals"] } diff --git a/src/config.rs b/src/config.rs index 30bf4082..d41774ae 100644 --- a/src/config.rs +++ b/src/config.rs @@ -284,6 +284,8 @@ mod tests { assert_eq!(load.target_file_size, ReadableSize::mb(1)); assert_eq!(load.purge_threshold, ReadableSize::mb(3)); assert_eq!(load.format_version, Version::V1); + assert_eq!(load.enable_log_recycle, false); + assert_eq!(load.prefill_for_recycle, false); load.sanitize().unwrap(); } @@ -297,7 +299,7 @@ mod tests { assert!(hard_load.sanitize().is_err()); let soft_error = r#" - recovery-read-block-size = "1KB" + recovery-read-block-size = 1 recovery-threads = 0 target-file-size = "5000MB" format-version = 2 @@ -305,6 +307,8 @@ mod tests { prefill-for-recycle = true "#; let soft_load: Config = toml::from_str(soft_error).unwrap(); + assert!(soft_load.recovery_read_block_size.0 < MIN_RECOVERY_READ_BLOCK_SIZE as u64); + assert!(soft_load.recovery_threads < MIN_RECOVERY_THREADS); let mut soft_sanitized = soft_load; soft_sanitized.sanitize().unwrap(); assert!(soft_sanitized.recovery_read_block_size.0 >= MIN_RECOVERY_READ_BLOCK_SIZE as u64); @@ -313,8 +317,6 @@ mod tests { soft_sanitized.purge_rewrite_threshold.unwrap(), soft_sanitized.target_file_size ); - assert_eq!(soft_sanitized.format_version, Version::V2); - assert!(soft_sanitized.enable_log_recycle); let recycle_error = r#" enable-log-recycle = true diff --git a/src/engine.rs b/src/engine.rs index 17321649..4467ca43 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -1324,6 +1324,7 @@ pub(crate) mod tests { engine.append(rid, index, index + 1, Some(&data)); } } + engine.append(11, 1, 11, Some(&data)); // The engine needs purge, and all old entries should be rewritten. assert!(engine @@ -1342,8 +1343,9 @@ pub(crate) mod tests { }); } - // Recover with rewrite queue and append queue. + engine.clean(11); let cleaned_region_ids = engine.memtables.cleaned_region_ids(); + assert_eq!(cleaned_region_ids.len(), 1); let engine = engine.reopen(); assert_eq!(engine.memtables.cleaned_region_ids(), cleaned_region_ids); @@ -2438,6 +2440,7 @@ pub(crate) mod tests { lb.drain(); }; { + // begin. let mut builder = AtomicGroupBuilder::with_id(3); builder.begin(&mut log_batch); log_batch.put(rid, key.clone(), value.clone()).unwrap(); @@ -2445,6 +2448,7 @@ pub(crate) mod tests { engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } { + // begin - unrelated - end. let mut builder = AtomicGroupBuilder::with_id(3); builder.begin(&mut log_batch); rid += 1; @@ -2464,6 +2468,7 @@ pub(crate) mod tests { engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } { + // begin - middle - middle - end. let mut builder = AtomicGroupBuilder::with_id(3); builder.begin(&mut log_batch); rid += 1; @@ -2488,6 +2493,7 @@ pub(crate) mod tests { engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } { + // begin - begin - end. let mut builder = AtomicGroupBuilder::with_id(3); builder.begin(&mut log_batch); rid += 1; @@ -2507,6 +2513,7 @@ pub(crate) mod tests { engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } { + // end - middle - end. // We must change id to avoid getting merged with last group. // It is actually not possible in real life to only have "begin" missing. let mut builder = AtomicGroupBuilder::with_id(4); @@ -2528,6 +2535,7 @@ pub(crate) mod tests { engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } { + // end - begin - end let mut builder = AtomicGroupBuilder::with_id(5); builder.begin(&mut LogBatch::default()); builder.end(&mut log_batch); @@ -2547,6 +2555,26 @@ pub(crate) mod tests { flush(&mut log_batch); engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); } + { + // begin - end - begin - end. + let mut builder = AtomicGroupBuilder::with_id(6); + builder.begin(&mut log_batch); + rid += 1; + log_batch.put(rid, key.clone(), value.clone()).unwrap(); + data.insert(rid); + flush(&mut log_batch); + builder.end(&mut log_batch); + flush(&mut log_batch); + let mut builder = AtomicGroupBuilder::with_id(7); + builder.begin(&mut log_batch); + flush(&mut log_batch); + builder.end(&mut log_batch); + rid += 1; + log_batch.put(rid, key.clone(), value.clone()).unwrap(); + data.insert(rid); + flush(&mut log_batch); + engine.pipe_log.rotate(LogQueue::Rewrite).unwrap(); + } engine.pipe_log.sync(LogQueue::Rewrite).unwrap(); let engine = engine.reopen(); diff --git a/src/env/default.rs b/src/env/default.rs index 9aadc591..44f4fa18 100644 --- a/src/env/default.rs +++ b/src/env/default.rs @@ -1,6 +1,8 @@ // Copyright (c) 2017-present, PingCAP, Inc. Licensed under Apache-2.0. -use std::io::{Error, ErrorKind, Read, Result as IoResult, Seek, SeekFrom, Write}; +#[cfg(feature = "failpoints")] +use std::io::{Error, ErrorKind}; +use std::io::{Read, Result as IoResult, Seek, SeekFrom, Write}; use std::path::Path; use std::sync::Arc; diff --git a/src/env/log_fd/plain.rs b/src/env/log_fd/plain.rs index 08c40135..03328e91 100644 --- a/src/env/log_fd/plain.rs +++ b/src/env/log_fd/plain.rs @@ -1,5 +1,8 @@ // Copyright (c) 2017-present, PingCAP, Inc. Licensed under Apache-2.0. +//! A naive file handle implementation based on standard `File`. All I/O +//! operations need to synchronize under a `RwLock`. + use crate::env::{Handle, Permission}; use fail::fail_point; diff --git a/src/file_pipe_log/pipe.rs b/src/file_pipe_log/pipe.rs index f42ade1b..09bc1f42 100644 --- a/src/file_pipe_log/pipe.rs +++ b/src/file_pipe_log/pipe.rs @@ -33,7 +33,6 @@ pub const DEFAULT_PATH_ID: PathId = 0; /// compatibility. pub const DEFAULT_FIRST_FILE_SEQ: FileSeq = 1; -#[derive(Debug)] pub struct File { pub seq: FileSeq, pub handle: Arc, diff --git a/src/log_batch.rs b/src/log_batch.rs index 33587ea9..d2bbaec2 100644 --- a/src/log_batch.rs +++ b/src/log_batch.rs @@ -383,11 +383,6 @@ impl LogItemBatch { self.items.drain(..) } - pub fn push(&mut self, item: LogItem) { - self.item_size += item.approximate_size(); - self.items.push(item); - } - pub fn merge(&mut self, rhs: &mut LogItemBatch) { for item in &mut rhs.items { if let LogItemContent::EntryIndexes(entry_indexes) = &mut item.content { @@ -1582,7 +1577,7 @@ mod tests { } #[test] - fn test_corruption() { + fn test_header_corruption() { let region_id = 7; let data = vec![b'x'; 16]; let mut batch = LogBatch::default(); @@ -1688,7 +1683,7 @@ mod tests { batch_handle.len = len; let file_context = LogFileContext::new(batch_handle.id, Version::V2); batch.prepare_write(&file_context).unwrap(); - // batch.finish_write(batch_handle); + assert_eq!(batch.approximate_size(), len); let encoded = batch.encoded_bytes(); assert_eq!(encoded.len(), len); let mut bytes_slice = encoded; diff --git a/src/memtable.rs b/src/memtable.rs index dee9dcf4..7a0ea41b 100644 --- a/src/memtable.rs +++ b/src/memtable.rs @@ -989,7 +989,7 @@ impl MemTableAccessor { /// [`MemTable`]s. /// /// This method is only used for recovery. - #[allow(dead_code)] + #[cfg(test)] pub fn cleaned_region_ids(&self) -> HashSet { let mut ids = HashSet::default(); let removed_memtables = self.removed_memtables.lock(); @@ -1202,7 +1202,6 @@ fn has_internal_key(item: &LogItem) -> bool { matches!(&item.content, LogItemContent::Kv(KeyValue { key, .. }) if crate::is_internal_key(key, None)) } -#[derive(Debug)] struct PendingAtomicGroup { status: AtomicGroupStatus, items: Vec, diff --git a/src/pipe_log.rs b/src/pipe_log.rs index 57e94d1e..33ca4071 100644 --- a/src/pipe_log.rs +++ b/src/pipe_log.rs @@ -118,7 +118,6 @@ impl Display for Version { } } -#[derive(Debug, Clone)] pub struct LogFileContext { pub id: FileId, pub version: Version, diff --git a/src/util.rs b/src/util.rs index 363ace71..2e35a83e 100644 --- a/src/util.rs +++ b/src/util.rs @@ -286,7 +286,7 @@ pub mod lz4 { } } else if !src.is_empty() { Err(Error::Corruption(format!( - "Content to compress to short {}", + "Content to compress too short {}", src.len() ))) } else { diff --git a/stress/Cargo.toml b/stress/Cargo.toml index e89c92a5..7fa0b830 100644 --- a/stress/Cargo.toml +++ b/stress/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stress" -version = "0.3.0" +version = "0.4.0" authors = ["The TiKV Authors"] edition = "2018"