Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to Slog #185

Merged
merged 8 commits into from
Jun 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,22 @@ lazy_static = "1.3.0"
prost = "0.5"
prost-derive = "0.5"
bytes = "0.4.11"
slog = "2.2"
quick-error = "1.2.2"
rand = "0.6.4"
hashbrown = "0.3"
fail = { version = "0.2", optional = true }
getset = "0.0.7"
slog-stdlog = "3.0.2"
slog-term = "2.4.0"
slog-envlogger = "2.1.0"

[dev-dependencies]
env_logger = "0.6"
criterion = ">0.2.4"
lazy_static = "1.0"
harness = { path = "harness" }
regex = "1.1"
slog-async = "2.3.0"

[[bench]]
name = "benches"
Expand Down
2 changes: 1 addition & 1 deletion benches/suites/progress_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn quick_progress_set(voters: usize, learners: usize) -> ProgressSet {
pub fn bench_progress_set_new(c: &mut Criterion) {
let bench = |b: &mut Bencher| {
// No setup.
b.iter(ProgressSet::new);
b.iter(|| ProgressSet::new());
};

c.bench_function("ProgressSet::new", bench);
Expand Down
14 changes: 3 additions & 11 deletions examples/five_mem_node/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.
#[macro_use]
extern crate log;
extern crate env_logger;
extern crate prost;
extern crate raft;
extern crate regex;

use std::collections::{HashMap, VecDeque};
use std::sync::mpsc::{self, Receiver, Sender, SyncSender, TryRecvError};
Expand All @@ -30,8 +24,6 @@ use raft::{prelude::*, StateRole};
use regex::Regex;

fn main() {
env_logger::init();

// Create 5 mailboxes to send/receive messages. Every node holds a `Receiver` to receive
// messages from others, and uses the respective `Sender` to send messages to others.
let (mut tx_vec, mut rx_vec) = (Vec::new(), Vec::new());
Expand Down Expand Up @@ -209,15 +201,15 @@ fn on_ready(
// Persistent raft logs. It's necessary because in `RawNode::advance` we stabilize
// raft logs to the latest position.
if let Err(e) = store.wl().append(ready.entries()) {
error!("persist raft log fail: {:?}, need to retry or panic", e);
eprintln!("persist raft log fail: {:?}, need to retry or panic", e);
return;
}

// Apply the snapshot. It's necessary because in `RawNode::advance` we stabilize the snapshot.
if *ready.snapshot() != Snapshot::default() {
let s = ready.snapshot().clone();
if let Err(e) = store.wl().apply_snapshot(s) {
error!("apply snapshot fail: {:?}, need to retry or panic", e);
eprintln!("apply snapshot fail: {:?}, need to retry or panic", e);
return;
}
}
Expand All @@ -226,7 +218,7 @@ fn on_ready(
for msg in ready.messages.drain(..) {
let to = msg.to;
if mailboxes[&to].send(msg).is_err() {
warn!("send raft message to {} fail, let Raft retry it", to);
eprintln!("send raft message to {} fail, let Raft retry it", to);
}
}

Expand Down
15 changes: 13 additions & 2 deletions examples/single_mem_node/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use raft;
#[macro_use]
extern crate slog;

use slog::Drain;
use std::collections::HashMap;
use std::sync::mpsc::{self, RecvTimeoutError};
use std::thread;
Expand Down Expand Up @@ -42,6 +44,15 @@ fn main() {
// Please check the Storage trait in src/storage.rs to see how to implement one.
let storage = MemStorage::new_with_conf_state(ConfState::from((vec![1], vec![])));

let decorator = slog_term::TermDecorator::new().build();
let drain = slog_term::FullFormat::new(decorator).build().fuse();
let drain = slog_async::Async::new(drain)
.chan_size(4096)
.overflow_strategy(slog_async::OverflowStrategy::Block)
.build()
.fuse();
let logger = slog::Logger::root(drain, o!());

// Create the configuration for the Raft node.
let cfg = Config {
// The unique ID for the Raft node.
Expand All @@ -66,7 +77,7 @@ fn main() {
};

// Create the Raft node.
let mut r = RawNode::new(&cfg, storage).unwrap();
let mut r = RawNode::new(&cfg, storage).unwrap().with_logger(&logger);

let (sender, receiver) = mpsc::channel();

Expand Down
4 changes: 3 additions & 1 deletion harness/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ edition = "2018"
[dependencies]
raft = { path = ".." }
rand = "0.6.4"
env_logger = "0.6"
slog = "2.2"
slog-term = "2.4.0"
slog-envlogger = "2.1.0"
27 changes: 24 additions & 3 deletions harness/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,34 @@ This module contains various testing harness utilities for Raft.

*/

#[macro_use]
extern crate slog;

mod interface;
mod network;

pub use self::{interface::Interface, network::Network};
use slog::{Drain, Logger};


/// Do any common test initialization. Eg set up logging.
/// Build a logger for tests.
///
/// Currently, this is a terminal log. It ensures it is only initialized once to prevent clobbering.
// This is `pub` so that testing and benching functions can use it.
// `#[cfg(test)]` doesn't work for benching.
#[doc(hidden)]
pub fn setup_for_test() {
nrc marked this conversation as resolved.
Show resolved Hide resolved
let _ = env_logger::try_init();
pub fn testing_logger() -> &'static Logger {
use std::sync::{Mutex, Once};
static LOGGER_INITIALIZED: Once = Once::new();
static mut LOGGER: Option<Logger> = None;

unsafe {
LOGGER_INITIALIZED.call_once(|| {
let decorator = slog_term::TermDecorator::new().build();
let drain = slog_term::CompactFormat::new(decorator).build().fuse();
let drain = slog_envlogger::new(drain).fuse();
LOGGER = Some(slog::Logger::root(Mutex::new(drain).fuse(), o!()));
});
LOGGER.as_ref().unwrap()
}
}
15 changes: 10 additions & 5 deletions harness/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use raft::{
Config, Raft, Result, NO_LIMIT,
};
use rand;
use slog::Logger;
use std::collections::HashMap;

/// A connection from one node to another.
Expand Down Expand Up @@ -78,13 +79,17 @@ impl Network {
///
/// A `None` node will be replaced with a new Raft node, and its configuration will
/// be `peers`.
pub fn new(peers: Vec<Option<Interface>>) -> Network {
pub fn new(peers: Vec<Option<Interface>>, l: &Logger) -> Network {
let config = Network::default_config();
Network::new_with_config(peers, &config)
Network::new_with_config(peers, &config, l)
}

/// Initialize a network from `peers` with explicitly specified `config`.
pub fn new_with_config(mut peers: Vec<Option<Interface>>, config: &Config) -> Network {
pub fn new_with_config(
mut peers: Vec<Option<Interface>>,
config: &Config,
l: &Logger,
) -> Network {
let mut nstorage = HashMap::new();
let mut npeers = HashMap::new();

Expand All @@ -97,8 +102,8 @@ impl Network {
nstorage.insert(*id, store.clone());
let mut config = config.clone();
config.id = *id;
config.tag = format!("{}", id);
let r = Raft::new(&config, store).unwrap().into();
config.tag = id.to_string();
let r = Raft::new(&config, store).unwrap().with_logger(l).into();
npeers.insert(*id, r);
}
Some(r) => {
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl Config {
pub fn new(id: u64) -> Self {
Self {
id,
tag: format!("{}", id),
tag: id.to_string(),
..Self::default()
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,10 @@ pub type Result<T> = result::Result<T, Error>;
#[cfg(test)]
mod tests {
use super::*;
use harness::setup_for_test;
use std::io;

#[test]
fn test_error_equal() {
setup_for_test();
assert_eq!(Error::StepPeerNotFound, Error::StepPeerNotFound);
assert_eq!(
Error::Store(StorageError::Compacted),
Expand Down Expand Up @@ -193,7 +191,6 @@ mod tests {

#[test]
fn test_storage_error_equal() {
setup_for_test();
assert_eq!(StorageError::Compacted, StorageError::Compacted);
assert_eq!(StorageError::Unavailable, StorageError::Unavailable);
assert_eq!(
Expand Down
22 changes: 21 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ before taking old, removed peers offline.
extern crate fail;

#[macro_use]
extern crate log;
extern crate slog;
#[macro_use]
extern crate quick_error;
#[macro_use]
Expand Down Expand Up @@ -412,6 +412,7 @@ pub use self::raw_node::{is_empty_snap, Peer, RawNode, Ready, SnapshotStatus};
pub use self::read_only::{ReadOnlyOption, ReadState};
pub use self::status::Status;
pub use self::storage::{RaftState, Storage};
use slog::{Drain, Logger};

pub mod prelude {
//! A "prelude" for crates using the `raft` crate.
Expand Down Expand Up @@ -444,3 +445,22 @@ pub mod prelude {

pub use crate::read_only::{ReadOnlyOption, ReadState};
}

/// 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.
#[doc(hidden)]
fn default_logger() -> &'static Logger {
use std::sync::{Mutex, Once};
static LOGGER_INITIALIZED: Once = Once::new();
static mut LOGGER: Option<Logger> = None;

unsafe {
LOGGER_INITIALIZED.call_once(|| {
let drain = slog_stdlog::StdLog.fuse();
let drain = slog_envlogger::new(drain).fuse();
LOGGER = Some(slog::Logger::root(Mutex::new(drain).fuse(), o!()));
});
LOGGER.as_ref().unwrap()
}
}
14 changes: 7 additions & 7 deletions src/log_unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl Unstable {
mod test {
use crate::eraftpb::{Entry, Snapshot, SnapshotMetadata};
use crate::log_unstable::Unstable;
use harness::setup_for_test;
use harness::testing_logger;

fn new_entry(index: u64, term: u64) -> Entry {
let mut e = Entry::default();
Expand All @@ -202,7 +202,7 @@ mod test {

#[test]
fn test_maybe_first_index() {
setup_for_test();
testing_logger().new(o!("test" => "maybe_first_index"));
// entry, offset, snap, wok, windex,
let tests = vec![
// no snapshot
Expand Down Expand Up @@ -230,7 +230,7 @@ mod test {

#[test]
fn test_maybe_last_index() {
setup_for_test();
testing_logger().new(o!("test" => "maybe_last_index"));
// entry, offset, snap, wok, windex,
let tests = vec![
(Some(new_entry(5, 1)), 5, None, true, 5),
Expand Down Expand Up @@ -258,7 +258,7 @@ mod test {

#[test]
fn test_maybe_term() {
setup_for_test();
testing_logger().new(o!("test" => "maybe_term"));
// entry, offset, snap, index, wok, wterm
let tests = vec![
// term from entries
Expand Down Expand Up @@ -320,7 +320,7 @@ mod test {

#[test]
fn test_restore() {
setup_for_test();
testing_logger().new(o!("test" => "restore"));
let mut u = Unstable {
entries: vec![new_entry(5, 1)],
offset: 5,
Expand All @@ -338,7 +338,7 @@ mod test {

#[test]
fn test_stable_to() {
setup_for_test();
testing_logger().new(o!("test" => "stable_to"));
// entries, offset, snap, index, term, woffset, wlen
let tests = vec![
(vec![], 0, None, 5, 1, 0, 0),
Expand Down Expand Up @@ -418,7 +418,7 @@ mod test {

#[test]
fn test_truncate_and_append() {
setup_for_test();
testing_logger().new(o!("test" => "truncate_and_append"));
// entries, offset, snap, to_append, woffset, wentries
let tests = vec![
// replace to the end
Expand Down
8 changes: 4 additions & 4 deletions src/progress/inflights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,11 @@ impl Inflights {
#[cfg(test)]
mod tests {
use super::Inflights;
use harness::setup_for_test;
use harness::testing_logger;

#[test]
fn test_inflight_add() {
setup_for_test();
let _ = testing_logger().new(o!("test" => "inflight_add"));
let mut inflight = Inflights::new(10);
for i in 0..5 {
inflight.add(i);
Expand Down Expand Up @@ -199,7 +199,7 @@ mod tests {

#[test]
fn test_inflight_free_to() {
setup_for_test();
let _ = testing_logger().new(o!("test" => "inflight_free_to"));
let mut inflight = Inflights::new(10);
for i in 0..10 {
inflight.add(i);
Expand Down Expand Up @@ -252,7 +252,7 @@ mod tests {

#[test]
fn test_inflight_free_first_one() {
setup_for_test();
let _ = testing_logger().new(o!("test" => "inflight_free_first_one"));
let mut inflight = Inflights::new(10);
for i in 0..10 {
inflight.add(i);
Expand Down
Loading