From 9a2374ae54d646697d28dc5bff035d8f1091fcd6 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Mon, 14 Oct 2024 10:32:41 +0700 Subject: [PATCH 1/6] fix: fix clippy Signed-off-by: Dat Tien Nguyen --- src/lib.rs | 22 +++++++++++----------- src/quorum/majority.rs | 2 +- src/raw_node.rs | 1 + src/storage.rs | 2 +- src/util.rs | 1 - 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 480c354b..10fb030a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -201,7 +201,7 @@ The `Ready` state contains quite a bit of information, and you need to check and by one: 1. Check whether `messages` is empty or not. If not, it means that the node will send messages to -other nodes: + other nodes: ```rust # use slog::{Drain, o}; @@ -226,7 +226,7 @@ other nodes: ``` 2. Check whether `snapshot` is empty or not. If not empty, it means that the Raft node has received -a Raft snapshot from the leader and we must apply the snapshot: + a Raft snapshot from the leader and we must apply the snapshot: ```rust # use slog::{Drain, o}; @@ -254,8 +254,8 @@ a Raft snapshot from the leader and we must apply the snapshot: ``` 3. Check whether `committed_entries` is empty or not. If not, it means that there are some newly -committed log entries which you must apply to the state machine. Of course, after applying, you -need to update the applied index and resume `apply` later: + committed log entries which you must apply to the state machine. Of course, after applying, you + need to update the applied index and resume `apply` later: ```rust # use slog::{Drain, o}; @@ -310,7 +310,7 @@ need to update the applied index and resume `apply` later: after restarting, *it may work but potential log loss may also be ignored silently*. 4. Check whether `entries` is empty or not. If not empty, it means that there are newly added -entries but have not been committed yet, we must append the entries to the Raft log: + entries but have not been committed yet, we must append the entries to the Raft log: ```rust # use slog::{Drain, o}; @@ -335,8 +335,8 @@ entries but have not been committed yet, we must append the entries to the Raft ``` 5. Check whether `hs` is empty or not. If not empty, it means that the `HardState` of the node has -changed. For example, the node may vote for a new leader, or the commit index has been increased. -We must persist the changed `HardState`: + changed. For example, the node may vote for a new leader, or the commit index has been increased. + We must persist the changed `HardState`: ```rust # use slog::{Drain, o}; @@ -360,7 +360,7 @@ We must persist the changed `HardState`: ``` 6. Check whether `persisted_messages` is empty or not. If not, it means that the node will send messages to -other nodes after persisting hardstate, entries and snapshot: + other nodes after persisting hardstate, entries and snapshot: ```rust # use slog::{Drain, o}; @@ -385,8 +385,8 @@ other nodes after persisting hardstate, entries and snapshot: ``` 7. Call `advance` to notify that the previous work is completed. Get the return value `LightReady` -and handle its `messages` and `committed_entries` like step 1 and step 3 does. Then call `advance_apply` -to advance the applied index inside. + and handle its `messages` and `committed_entries` like step 1 and step 3 does. Then call `advance_apply` + to advance the applied index inside. ```rust # use slog::{Drain, o}; @@ -469,7 +469,7 @@ node.propose_conf_change(vec![], cc).unwrap(); This process is a two-phase process, during the midst of it the peer group's leader is managing **two independent, possibly overlapping peer sets**. -> **Note:** In order to maintain resiliency guarantees (progress while a majority of both peer sets is +\> **Note:** In order to maintain resiliency guarantees (progress while a majority of both peer sets is active), it is recommended to wait until the entire peer group has exited the transition phase before taking old, removed peers offline. diff --git a/src/quorum/majority.rs b/src/quorum/majority.rs index 5fcd103e..4ea9292e 100644 --- a/src/quorum/majority.rs +++ b/src/quorum/majority.rs @@ -7,7 +7,7 @@ use std::collections::hash_set::Iter; use std::fmt::Formatter; use std::mem::MaybeUninit; use std::ops::{Deref, DerefMut}; -use std::{cmp, slice, u64}; +use std::{cmp, slice}; /// A set of IDs that uses majority quorums to make decisions. #[derive(Clone, Debug, Default, PartialEq, Eq)] diff --git a/src/raw_node.rs b/src/raw_node.rs index 55b28696..b8adbcc2 100644 --- a/src/raw_node.rs +++ b/src/raw_node.rs @@ -218,6 +218,7 @@ impl Ready { /// MustSync is false if and only if /// 1. no HardState or only its commit is different from before /// 2. no Entries and Snapshot + /// /// If it's false, an asynchronous write of HardState is permissible before calling /// [`RawNode::on_persist_ready`] or [`RawNode::advance`] or its families. #[inline] diff --git a/src/storage.rs b/src/storage.rs index 288501e0..99e02e3a 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -579,7 +579,7 @@ mod test { new_entry(5, 5), new_entry(6, 6), ]; - let max_u64 = u64::max_value(); + let max_u64 = u64::MAX; let mut tests = vec![ ( 2, diff --git a/src/util.rs b/src/util.rs index 9ad603d2..c4fd582f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,7 +5,6 @@ use std::fmt; use std::fmt::Write; -use std::u64; use slog::{OwnedKVList, Record, KV}; From 88e5f46faa898b3d787bf48d2b8d2a071e9ad693 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Mon, 14 Oct 2024 11:15:35 +0700 Subject: [PATCH 2/6] fix: bypass clippy::manual_inspect because of false-positive Signed-off-by: Dat Tien Nguyen --- datadriven/src/datadriven.rs | 2 ++ datadriven/src/test_data_reader.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/datadriven/src/datadriven.rs b/datadriven/src/datadriven.rs index cd3c942e..9f95c380 100644 --- a/datadriven/src/datadriven.rs +++ b/datadriven/src/datadriven.rs @@ -15,6 +15,7 @@ use lazy_static::lazy_static; use similar_asserts::assert_eq; use slog::debug; + /// The main function to run tests /// /// You need to pass the path of `testdata` where store the test cases, and your function @@ -138,6 +139,7 @@ where // run_directive runs just one directive in the input. // +#[allow(clippy::manual_inspect)] fn run_directive(r: &mut TestDataReader, mut f: F) where F: FnMut(&TestData) -> String, diff --git a/datadriven/src/test_data_reader.rs b/datadriven/src/test_data_reader.rs index e5d6e2a6..6b1be754 100644 --- a/datadriven/src/test_data_reader.rs +++ b/datadriven/src/test_data_reader.rs @@ -205,6 +205,7 @@ impl<'a> TestDataReader<'a> { } } + #[allow(clippy::manual_inspect)] pub fn emit(&mut self, str: &str) { self.rewrite_buffer.as_mut().map(|rb| { let str = str.to_string() + "\n"; From b28e89901b28640de0563eea7cb7abb604000cb0 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Mon, 14 Oct 2024 18:04:15 +0700 Subject: [PATCH 3/6] fix: run cargo fmt Signed-off-by: Dat Tien Nguyen --- datadriven/src/datadriven.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/datadriven/src/datadriven.rs b/datadriven/src/datadriven.rs index 9f95c380..f3778441 100644 --- a/datadriven/src/datadriven.rs +++ b/datadriven/src/datadriven.rs @@ -15,7 +15,6 @@ use lazy_static::lazy_static; use similar_asserts::assert_eq; use slog::debug; - /// The main function to run tests /// /// You need to pass the path of `testdata` where store the test cases, and your function From 465c5cdd3f604ba98813bdc55f770468f6419b64 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Tue, 15 Oct 2024 22:08:07 +0700 Subject: [PATCH 4/6] fix: add allow `static_mut_refs` Signed-off-by: Dat Tien Nguyen --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 10fb030a..0b2879e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -571,6 +571,7 @@ pub mod prelude { /// /// Currently, this is a `log` adaptor behind a `Once` to ensure there is no clobbering. #[cfg(any(test, feature = "default-logger"))] +#[allow(static_mut_refs)] pub fn default_logger() -> slog::Logger { use slog::{o, Drain}; use std::sync::{Mutex, Once}; From baeae3a09291aab4c7a337322cc167c30b9af200 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Tue, 15 Oct 2024 22:39:16 +0700 Subject: [PATCH 5/6] fix(nightly-rust): fix clippy for too long opening paragraph. Signed-off-by: Dat Tien Nguyen --- src/confchange/changer.rs | 4 +++- src/log_unstable.rs | 3 +++ src/read_only.rs | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/confchange/changer.rs b/src/confchange/changer.rs index 32e23170..3c3e1112 100644 --- a/src/confchange/changer.rs +++ b/src/confchange/changer.rs @@ -33,7 +33,9 @@ impl IncrChangeMap<'_> { } } -/// Changer facilitates configuration changes. It exposes methods to handle +/// Changer facilitates configuration changes. +/// +/// It exposes methods to handle /// simple and joint consensus while performing the proper validation that allows /// refusing invalid configuration changes before they affect the active /// configuration. diff --git a/src/log_unstable.rs b/src/log_unstable.rs index e27b9cf6..dd90615e 100644 --- a/src/log_unstable.rs +++ b/src/log_unstable.rs @@ -20,6 +20,9 @@ use crate::eraftpb::{Entry, Snapshot}; use crate::util::entry_approximate_size; use slog::Logger; +/// Unstable contains "unstable" log entries and snapshot state that has +/// not yet been written to Storage. +/// /// The `unstable.entries[i]` has raft log position `i+unstable.offset`. /// Note that `unstable.offset` may be less than the highest log /// position in storage; this means that the next write to storage diff --git a/src/read_only.rs b/src/read_only.rs index 6ae37282..1dd0ffa4 100644 --- a/src/read_only.rs +++ b/src/read_only.rs @@ -37,6 +37,7 @@ pub enum ReadOnlyOption { } /// ReadState provides state for read only query. +/// /// It's caller's responsibility to send MsgReadIndex first before getting /// this state from ready. It's also caller's duty to differentiate if this /// state is what it requests through request_ctx, e.g. given a unique id as From ffe7c9ae3bd0d96313299b4d641b1270fc341dd1 Mon Sep 17 00:00:00 2001 From: Dat Tien Nguyen Date: Sun, 20 Oct 2024 12:09:09 +0700 Subject: [PATCH 6/6] fix: remove some `allow` directive Signed-off-by: Dat Tien Nguyen --- datadriven/src/datadriven.rs | 6 ++---- datadriven/src/test_data_reader.rs | 6 ++---- src/lib.rs | 33 +++++++++++++----------------- 3 files changed, 18 insertions(+), 27 deletions(-) diff --git a/datadriven/src/datadriven.rs b/datadriven/src/datadriven.rs index f3778441..64dda20b 100644 --- a/datadriven/src/datadriven.rs +++ b/datadriven/src/datadriven.rs @@ -138,7 +138,6 @@ where // run_directive runs just one directive in the input. // -#[allow(clippy::manual_inspect)] fn run_directive(r: &mut TestDataReader, mut f: F) where F: FnMut(&TestData) -> String, @@ -157,10 +156,9 @@ where if has_blank_line(&actual) { r.emit("----"); - r.rewrite_buffer.as_mut().map(|rb| { + if let Some(rb) = r.rewrite_buffer.as_mut() { rb.push_str(&actual); - rb - }); + } r.emit("----"); r.emit("----"); diff --git a/datadriven/src/test_data_reader.rs b/datadriven/src/test_data_reader.rs index 6b1be754..9d11d22c 100644 --- a/datadriven/src/test_data_reader.rs +++ b/datadriven/src/test_data_reader.rs @@ -205,12 +205,10 @@ impl<'a> TestDataReader<'a> { } } - #[allow(clippy::manual_inspect)] pub fn emit(&mut self, str: &str) { - self.rewrite_buffer.as_mut().map(|rb| { + if let Some(rb) = self.rewrite_buffer.as_mut() { let str = str.to_string() + "\n"; rb.push_str(&str); - rb - }); + } } } diff --git a/src/lib.rs b/src/lib.rs index 0b2879e4..fc01400c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -469,9 +469,9 @@ node.propose_conf_change(vec![], cc).unwrap(); This process is a two-phase process, during the midst of it the peer group's leader is managing **two independent, possibly overlapping peer sets**. -\> **Note:** In order to maintain resiliency guarantees (progress while a majority of both peer sets is -active), it is recommended to wait until the entire peer group has exited the transition phase -before taking old, removed peers offline. +> **Note:** In order to maintain resiliency guarantees (progress while a majority of both peer sets is +> active), it is recommended to wait until the entire peer group has exited the transition phase +> before taking old, removed peers offline. */ @@ -569,25 +569,20 @@ pub mod prelude { /// The default logger we fall back to when passed `None` in external facing constructors. /// -/// Currently, this is a `log` adaptor behind a `Once` to ensure there is no clobbering. +/// Currently, this is a `log` adaptor behind a `OnceLock` to ensure there is no clobbering. #[cfg(any(test, feature = "default-logger"))] -#[allow(static_mut_refs)] pub fn default_logger() -> slog::Logger { use slog::{o, Drain}; - use std::sync::{Mutex, Once}; - - static LOGGER_INITIALIZED: Once = Once::new(); - static mut LOGGER: Option = None; - - let logger = unsafe { - LOGGER_INITIALIZED.call_once(|| { - let decorator = slog_term::TermDecorator::new().build(); - let drain = slog_term::CompactFormat::new(decorator).build(); - let drain = slog_envlogger::new(drain); - LOGGER = Some(slog::Logger::root(Mutex::new(drain).fuse(), o!())); - }); - LOGGER.as_ref().unwrap() - }; + use std::sync::{Mutex, OnceLock}; + + static LOGGER_INITIALIZED: OnceLock = OnceLock::new(); + let logger = LOGGER_INITIALIZED.get_or_init(|| { + let decorator = slog_term::TermDecorator::new().build(); + let drain = slog_term::CompactFormat::new(decorator).build(); + let drain = slog_envlogger::new(drain); + slog::Logger::root(Mutex::new(drain).fuse(), o!()) + }); + if let Some(case) = std::thread::current() .name() .and_then(|v| v.split(':').last())