From e3c788e4c05afa89de45dd8b7c6ee6c9f22c22a2 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 23 Jan 2020 21:35:34 -0600 Subject: [PATCH 01/16] Don't use PID under miri for temporary file names --- src/config.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/config.rs b/src/config.rs index 20fc29cba..41c3bbfa0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -455,8 +455,12 @@ impl Config { .as_nanos() << 48; + #[cfg(not(miri))] let pid = u128::from(std::process::id()); + #[cfg(miri)] + let pid = 0; + let salt = (pid << 16) + now + seed; if cfg!(target_os = "linux") { From 45be29ee962f0d2fd8b8c7bb2ae9b3cd8b64dbb1 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 23 Jan 2020 21:42:04 -0600 Subject: [PATCH 02/16] Ignore quickcheck tests under miri --- src/fastcmp.rs | 2 ++ src/serialization.rs | 12 ++++++++++++ tests/test_tree.rs | 2 +- tests/test_tree_failpoints.rs | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/fastcmp.rs b/src/fastcmp.rs index a0618f214..c615b7597 100644 --- a/src/fastcmp.rs +++ b/src/fastcmp.rs @@ -46,7 +46,9 @@ mod qc { prop_cmp_matches(pair[0], pair[1]); } } + quickcheck::quickcheck! { + #[cfg_attr(miri, ignore)] fn qc_fastcmp(l: Vec, r: Vec) -> bool { prop_cmp_matches(&l, &r) } diff --git a/src/serialization.rs b/src/serialization.rs index 7b7d4be52..42079c057 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -1043,50 +1043,62 @@ mod qc { } quickcheck::quickcheck! { + #[cfg_attr(miri, ignore)] fn bool(item: bool) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn u8(item: u8) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn i64(item: SpreadI64) -> bool { prop_serialize(item.0) } + #[cfg_attr(miri, ignore)] fn u64(item: SpreadU64) -> bool { prop_serialize(item.0) } + #[cfg_attr(miri, ignore)] fn disk_ptr(item: DiskPtr) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn page_state(item: PageState) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn meta(item: Meta) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn snapshot(item: Snapshot) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn node(item: Node) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn data(item: Data) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn link(item: Link) -> bool { prop_serialize(item) } + #[cfg_attr(miri, ignore)] fn msg_header(item: MessageHeader) -> bool { prop_serialize(item) } diff --git a/tests/test_tree.rs b/tests/test_tree.rs index 717ed87d4..3b2ea7e32 100644 --- a/tests/test_tree.rs +++ b/tests/test_tree.rs @@ -872,7 +872,7 @@ fn tree_import_export() -> Result<()> { } #[test] -#[cfg(not(target_os = "fuchsia"))] +#[cfg_attr(any(target_os = "fuchsia", miri), ignore)] fn quickcheck_tree_matches_btreemap() { let n_tests = if cfg!(windows) { 25 } else { 100 }; diff --git a/tests/test_tree_failpoints.rs b/tests/test_tree_failpoints.rs index 8859f2219..cb4004ff7 100644 --- a/tests/test_tree_failpoints.rs +++ b/tests/test_tree_failpoints.rs @@ -500,7 +500,7 @@ fn run_tree_crashes_nicely(ops: Vec, flusher: bool) -> bool { } #[test] -#[cfg(not(target_os = "fuchsia"))] +#[cfg_attr(any(target_os = "fuchsia", miri), ignore)] fn quickcheck_tree_with_failpoints() { // use fewer tests for travis OSX builds that stall out all the time let mut n_tests = 50; From 822f26ffacd07d03f09bb2991207f5fb8e5fc620 Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 24 Jan 2020 07:18:31 -0600 Subject: [PATCH 03/16] Don't use FileExt::lock_exclusive under miri --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 41c3bbfa0..3f374700e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -577,7 +577,7 @@ impl Config { } fn try_lock(&self, file: File) -> Result { - #[cfg(any(windows, target_os = "linux", target_os = "macos"))] + #[cfg(all(not(miri), any(windows, target_os = "linux", target_os = "macos")))] { use fs2::FileExt; From c11308a9385fd6aebddfb95dc27cb9a04b6ac645 Mon Sep 17 00:00:00 2001 From: David Cook Date: Sat, 25 Jan 2020 22:59:39 -0600 Subject: [PATCH 04/16] Disable threadpool under miri --- src/context.rs | 12 ++++++------ src/db.rs | 4 ++-- src/lib.rs | 12 ++++++------ src/pagecache/mod.rs | 18 ++++++++++++++++++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/context.rs b/src/context.rs index a90f18a80..b0e132987 100644 --- a/src/context.rs +++ b/src/context.rs @@ -12,7 +12,7 @@ pub struct Context { /// When the last high-level reference is dropped, it /// should trigger all background threads to clean /// up synchronously. - #[cfg(any( + #[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -20,7 +20,7 @@ pub struct Context { target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", - ))] + )))] pub(crate) flusher: Arc>>, #[doc(hidden)] pub pagecache: Arc, @@ -36,7 +36,7 @@ impl std::ops::Deref for Context { impl Drop for Context { fn drop(&mut self) { - #[cfg(any( + #[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -44,7 +44,7 @@ impl Drop for Context { target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", - ))] + )))] { if let Some(flusher) = self.flusher.lock().take() { drop(flusher) @@ -77,7 +77,7 @@ impl Context { Ok(Self { config, pagecache, - #[cfg(any( + #[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -85,7 +85,7 @@ impl Context { target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", - ))] + )))] flusher: Arc::new(parking_lot::Mutex::new(None)), }) } diff --git a/src/db.rs b/src/db.rs index de37d4ab7..b20c7fa0d 100644 --- a/src/db.rs +++ b/src/db.rs @@ -69,7 +69,7 @@ impl Db { let context = Context::start(config)?; - #[cfg(any( + #[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -77,7 +77,7 @@ impl Db { target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", - ))] + )))] { let flusher_pagecache = context.pagecache.clone(); let flusher = context.flush_every_ms.map(move |fem| { diff --git a/src/lib.rs b/src/lib.rs index 787db784d..83b4f70ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,7 +209,7 @@ pub mod fail; #[cfg(feature = "docs")] pub mod doc; -#[cfg(not(any( +#[cfg(any(miri, not(any( windows, target_os = "linux", target_os = "macos", @@ -217,7 +217,7 @@ pub mod doc; target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", -)))] +))))] mod threadpool { use super::OneShot; @@ -233,7 +233,7 @@ mod threadpool { } } -#[cfg(any( +#[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -241,10 +241,10 @@ mod threadpool { target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", -))] +)))] mod threadpool; -#[cfg(any( +#[cfg(all(not(miri), any( windows, target_os = "linux", target_os = "macos", @@ -252,7 +252,7 @@ mod threadpool; target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", -))] +)))] mod flusher; #[cfg(feature = "event_log")] diff --git a/src/pagecache/mod.rs b/src/pagecache/mod.rs index 2a4ebcb81..c2b2fa3ff 100644 --- a/src/pagecache/mod.rs +++ b/src/pagecache/mod.rs @@ -824,6 +824,15 @@ impl PageCache { /// move a page. Returns Ok(false) if there were no pages /// to GC. Returns an Err if we encountered an IO problem /// while performing this GC. + #[cfg(all(not(miri), any( + windows, + target_os = "linux", + target_os = "macos", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + )))] pub(crate) fn attempt_gc(&self) -> Result { let guard = pin(); let cc = concurrency_control::read(); @@ -881,6 +890,15 @@ impl PageCache { #[doc(hidden)] #[cfg(feature = "failpoints")] + #[cfg(all(not(miri), any( + windows, + target_os = "linux", + target_os = "macos", + target_os = "dragonfly", + target_os = "freebsd", + target_os = "openbsd", + target_os = "netbsd", + )))] pub(crate) fn set_failpoint(&self, e: Error) { if let Error::FailPoint = e { self.config.set_global_error(e); From 07b5065e72e6cb9039322ab1478b719014720754 Mon Sep 17 00:00:00 2001 From: David Cook Date: Sat, 25 Jan 2020 23:17:48 -0600 Subject: [PATCH 05/16] Disable tests that require threads under miri --- src/stack.rs | 1 + tests/test_crash_recovery.rs | 6 ++++++ tests/test_log.rs | 5 +++-- tests/test_quiescent.rs | 2 +- tests/test_tree.rs | 17 ++++++++++++++--- 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/stack.rs b/src/stack.rs index 16fc06122..4e4e8ecb9 100644 --- a/src/stack.rs +++ b/src/stack.rs @@ -221,6 +221,7 @@ where } #[test] +#[cfg(not(miri))] // can't create threads fn basic_functionality() { use crossbeam_epoch::pin; use crossbeam_utils::CachePadded; diff --git a/tests/test_crash_recovery.rs b/tests/test_crash_recovery.rs index c147b5dee..c1c95bba5 100644 --- a/tests/test_crash_recovery.rs +++ b/tests/test_crash_recovery.rs @@ -23,6 +23,12 @@ const RECOVERY_NO_SNAPSHOT: &str = "crash_recovery_no_runtime_snapshot"; const BATCHES_NO_SNAPSHOT: &str = "crash_batches_no_runtime_snapshot"; fn main() { + // Don't actually run this harness=false test under miri, as it requires spawning and killing + // child processes. + if cfg!(miri) { + return; + } + common::setup_logger(); match env::var(TEST_ENV_VAR) { diff --git a/tests/test_log.rs b/tests/test_log.rs index 4ce062db8..532f06007 100644 --- a/tests/test_log.rs +++ b/tests/test_log.rs @@ -1,7 +1,5 @@ mod common; -use std::thread; - use rand::{thread_rng, Rng}; use sled::*; @@ -111,7 +109,10 @@ fn non_contiguous_log_flush() -> Result<()> { } #[test] +#[cfg(not(miri))] // can't create threads fn concurrent_logging() -> Result<()> { + use std::thread; + common::setup_logger(); for _ in 0..10 { let config = Config::new() diff --git a/tests/test_quiescent.rs b/tests/test_quiescent.rs index 4240cb2c7..251b280af 100644 --- a/tests/test_quiescent.rs +++ b/tests/test_quiescent.rs @@ -1,4 +1,4 @@ -#![cfg(target_os = "linux")] +#![cfg(all(target_os = "linux", not(miri)))] mod common; diff --git a/tests/test_tree.rs b/tests/test_tree.rs index 3b2ea7e32..9bb58c752 100644 --- a/tests/test_tree.rs +++ b/tests/test_tree.rs @@ -1,9 +1,9 @@ mod common; mod tree; -use std::sync::{Arc, Barrier}; -use std::thread; +use std::sync::Arc; +#[allow(unused_imports)] use log::{debug, warn}; use quickcheck::{QuickCheck, StdGen}; @@ -20,6 +20,7 @@ const N_PER_THREAD: usize = 100; const N: usize = N_THREADS * N_PER_THREAD; // NB N should be multiple of N_THREADS const SPACE: usize = N; +#[allow(dead_code)] const INTENSITY: usize = 5; fn kv(i: usize) -> Vec { @@ -49,7 +50,7 @@ fn very_large_reverse_tree_iterator() { } #[test] -#[cfg(target_os = "linux")] +#[cfg(all(target_os = "linux", not(miri)))] fn test_varied_compression_ratios() { // tests for the compression issue reported in #938 by @Mrmaxmeier. @@ -85,7 +86,10 @@ fn test_varied_compression_ratios() { } #[test] +#[cfg(not(miri))] // can't create threads fn concurrent_tree_ops() { + use std::thread; + common::setup_logger(); for i in 0..INTENSITY { @@ -209,7 +213,11 @@ fn concurrent_tree_ops() { } #[test] +#[cfg(not(miri))] // can't create threads fn concurrent_tree_iter() -> Result<()> { + use std::sync::Barrier; + use std::thread; + common::setup_logger(); const N_FORWARD: usize = INTENSITY; @@ -381,7 +389,10 @@ fn concurrent_tree_iter() -> Result<()> { } #[test] +#[cfg(not(miri))] // can't create threads fn concurrent_tree_transactions() -> TransactionResult<()> { + use std::sync::Barrier; + common::setup_logger(); let config = Config::new() From 65f3e60752a780f8c508e506ec9f6b635ea80847 Mon Sep 17 00:00:00 2001 From: David Cook Date: Sun, 26 Jan 2020 00:17:45 -0600 Subject: [PATCH 06/16] Disable use of pwrite/pread under miri --- src/pagecache/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pagecache/mod.rs b/src/pagecache/mod.rs index c2b2fa3ff..38bfd5def 100644 --- a/src/pagecache/mod.rs +++ b/src/pagecache/mod.rs @@ -10,11 +10,11 @@ mod disk_pointer; mod iobuf; mod iterator; mod pagetable; -#[cfg(all(not(unix), not(windows)))] +#[cfg(any(all(not(unix), not(windows)), miri))] mod parallel_io_polyfill; -#[cfg(unix)] +#[cfg(all(unix, not(miri)))] mod parallel_io_unix; -#[cfg(windows)] +#[cfg(all(windows, not(miri)))] mod parallel_io_windows; mod reservation; mod segment; @@ -24,13 +24,13 @@ use std::{collections::BinaryHeap, ops::Deref}; use crate::*; -#[cfg(all(not(unix), not(windows)))] +#[cfg(any(all(not(unix), not(windows)), miri))] use parallel_io_polyfill::{pread_exact, pread_exact_or_eof, pwrite_all}; -#[cfg(unix)] +#[cfg(all(unix, not(miri)))] use parallel_io_unix::{pread_exact, pread_exact_or_eof, pwrite_all}; -#[cfg(windows)] +#[cfg(all(windows, not(miri)))] use parallel_io_windows::{pread_exact, pread_exact_or_eof, pwrite_all}; use self::{ From fe0294c0219e379d1d5b268d7f72f82b0ddd90ce Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 13 Apr 2020 17:23:10 -0500 Subject: [PATCH 07/16] Add smoke test for Lru --- src/lru.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/lru.rs b/src/lru.rs index 1c3cf78eb..65149d7f4 100644 --- a/src/lru.rs +++ b/src/lru.rs @@ -346,3 +346,14 @@ impl Shard { fn safe_usize(value: PageId) -> usize { usize::try_from(value).unwrap() } + +#[test] +fn lru_smoke_test() { + use crate::pin; + + let lru = Lru::new(256); + for i in 0..1000 { + let guard = pin(); + lru.accessed(i, 16, &guard); + } +} From 7b38c3ade7b46ae1483042992a115c28f8e07877 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 13 Apr 2020 17:59:44 -0500 Subject: [PATCH 08/16] Fix stacked borrows violation in CacheAccessIter The pointer `self.current_block` is separately turned into a shared reference and a Box in the same method, and moreover, the reference is created before the Box and used after the Box. This results in a unique reference from the Box and a shared reference from a separate pointer-to-reference conversion coexisting, with different tags. To fix this issue, the conversion from a raw pointer to a Box is put off until the shared reference is no longer in use, so that the two different tags won't pop each other off the stack. --- src/lru.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lru.rs b/src/lru.rs index 65149d7f4..521f73dc1 100644 --- a/src/lru.rs +++ b/src/lru.rs @@ -156,11 +156,12 @@ impl<'a> Iterator for CacheAccessIter<'a> { debug_delay(); if self.current_offset >= MAX_QUEUE_ITEMS { - let to_drop = unsafe { Box::from_raw(self.current_block) }; + let to_drop_ptr = self.current_block; debug_delay(); self.current_block = current_block.next.load(Ordering::Acquire); self.current_offset = 0; debug_delay(); + let to_drop = unsafe { Box::from_raw(to_drop_ptr) }; self.guard.defer(|| to_drop); continue; } From 7b873b4a4fcdf70e01d7f5289f26c8c6e8010cdf Mon Sep 17 00:00:00 2001 From: David Cook Date: Sun, 19 Apr 2020 10:13:51 -0500 Subject: [PATCH 09/16] Exclude compression in tests Can't use compression because zstd relies on calling a native C library --- tests/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tree/mod.rs b/tests/tree/mod.rs index 636be2538..39611fb17 100644 --- a/tests/tree/mod.rs +++ b/tests/tree/mod.rs @@ -58,7 +58,7 @@ impl RngCore for SledGen { pub fn fuzz_then_shrink(buf: &[u8]) { let use_compression = - cfg!(feature = "compression") && buf.get(0).unwrap_or(&0) % 2 == 0; + cfg!(feature = "compression") && !cfg!(miri) && buf.get(0).unwrap_or(&0) % 2 == 0; let ops: Vec = buf .chunks(2) From 83cd2e9666b9e2155168f963cde21a0c904480ca Mon Sep 17 00:00:00 2001 From: David Cook Date: Sun, 19 Apr 2020 10:15:13 -0500 Subject: [PATCH 10/16] Conditionally ignore more long-running tests --- tests/test_log.rs | 2 +- tests/test_space_leaks.rs | 1 + tests/test_tree.rs | 50 +++++++++++++++++++++++++++++++++++ tests/test_tree_failpoints.rs | 35 ++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/tests/test_log.rs b/tests/test_log.rs index 532f06007..88290c54c 100644 --- a/tests/test_log.rs +++ b/tests/test_log.rs @@ -280,7 +280,7 @@ fn log_aborts() { } #[test] -#[cfg(not(target_os = "fuchsia"))] +#[cfg_attr(any(target_os = "fuchsia", miri), ignore)] fn log_chunky_iterator() { common::setup_logger(); let config = diff --git a/tests/test_space_leaks.rs b/tests/test_space_leaks.rs index 34ea13750..214c37b4b 100644 --- a/tests/test_space_leaks.rs +++ b/tests/test_space_leaks.rs @@ -1,6 +1,7 @@ mod common; #[test] +#[cfg_attr(miri, ignore)] fn size_leak() -> sled::Result<()> { common::setup_logger(); diff --git a/tests/test_tree.rs b/tests/test_tree.rs index 9bb58c752..68720f55a 100644 --- a/tests/test_tree.rs +++ b/tests/test_tree.rs @@ -5,6 +5,7 @@ use std::sync::Arc; #[allow(unused_imports)] use log::{debug, warn}; + use quickcheck::{QuickCheck, StdGen}; use sled::Transactional; @@ -30,6 +31,7 @@ fn kv(i: usize) -> Vec { } #[test] +#[cfg_attr(miri, ignore)] fn very_large_reverse_tree_iterator() { let mut a = vec![255; 1024 * 1024]; a.push(0); @@ -769,6 +771,7 @@ fn tree_range() { } #[test] +#[cfg_attr(miri, ignore)] fn recover_tree() { common::setup_logger(); @@ -898,12 +901,14 @@ fn quickcheck_tree_matches_btreemap() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_00() { // postmortem: prop_tree_matches_btreemap(vec![Restart], false, false, 0, 0); } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_01() { // postmortem: // this was a bug in the snapshot recovery, where @@ -929,6 +934,7 @@ fn tree_bug_01() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_02() { // postmortem: // this was a bug in the way that the `Materializer` @@ -957,6 +963,7 @@ fn tree_bug_02() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_03() { // postmortem: the tree was not persisting and recovering root hoists // postmortem 2: when refactoring the log storage, we failed to restart @@ -981,6 +988,7 @@ fn tree_bug_03() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_04() { // postmortem: pagecache was failing to replace the LogId list // when it encountered a new Update::Compact. @@ -1007,6 +1015,7 @@ fn tree_bug_04() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_05() { // postmortem: during recovery, the segment accountant was failing to // properly set the file's tip. @@ -1029,6 +1038,7 @@ fn tree_bug_05() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_06() { // postmortem: after reusing segments, we were failing to checksum reads // performed while iterating over rewritten segment buffers, and using @@ -1053,6 +1063,7 @@ fn tree_bug_06() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_07() { // postmortem: the segment accountant was not fully recovered, and thought // that it could reuse a particular segment that wasn't actually empty @@ -1077,6 +1088,7 @@ fn tree_bug_07() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_08() { // postmortem: failed to properly recover the state in the segment // accountant that tracked the previously issued segment. @@ -1100,6 +1112,7 @@ fn tree_bug_08() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_09() { // postmortem: was failing to load existing snapshots on initialization. // would encounter uninitialized segments at the log tip and overwrite @@ -1126,6 +1139,7 @@ fn tree_bug_09() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_10() { // postmortem: after reusing a segment, but not completely writing a // segment, we were hitting an old LSN and violating an assert, rather @@ -1165,6 +1179,7 @@ fn tree_bug_10() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_11() { // postmortem: a stall was happening because LSNs and LogIds were being // conflated in calls to make_stable. A higher LogId than any LSN was @@ -1191,6 +1206,7 @@ fn tree_bug_11() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_12() { // postmortem: was not checking that a log entry's LSN matches its position // as part of detecting tears / partial rewrites. @@ -1242,6 +1258,7 @@ fn tree_bug_12() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_13() { // postmortem: failed root hoists were being improperly recovered before the // following free was done on their page, but we treated the written node as @@ -1273,6 +1290,7 @@ fn tree_bug_13() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_14() { // postmortem: after adding prefix compression, we were not // handling re-inserts and deletions properly @@ -1294,6 +1312,7 @@ fn tree_bug_14() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_15() { // postmortem: was not sorting keys properly when binary searching for them prop_tree_matches_btreemap( @@ -1313,6 +1332,7 @@ fn tree_bug_15() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_16() { // postmortem: the test merge function was not properly adding numbers. prop_tree_matches_btreemap( @@ -1325,6 +1345,7 @@ fn tree_bug_16() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_17() { // postmortem: we were creating a copy of a node leaf during iteration // before accidentally putting it into a PinnedValue, despite the @@ -1342,6 +1363,7 @@ fn tree_bug_17() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_18() { // postmortem: when implementing get_gt and get_lt, there were some // issues with getting order comparisons correct. @@ -1362,6 +1384,7 @@ fn tree_bug_18() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_19() { // postmortem: we were not seeking properly to the next node // when we hit a half-split child and were using get_lt @@ -1382,6 +1405,7 @@ fn tree_bug_19() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_20() { // postmortem: we were not seeking forward during get_gt // if path_for_key reached a leaf that didn't include @@ -1403,6 +1427,7 @@ fn tree_bug_20() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_21() { // postmortem: more split woes while implementing get_lt // postmortem 2: failed to properly account for node hi key @@ -1425,6 +1450,7 @@ fn tree_bug_21() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_22() { // postmortem: inclusivity wasn't being properly flipped off after // the first result during iteration @@ -1443,6 +1469,7 @@ fn tree_bug_22() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_23() { // postmortem: when rewriting CRC handling code, mis-sized the blob crc prop_tree_matches_btreemap( @@ -1455,6 +1482,7 @@ fn tree_bug_23() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_24() { // postmortem: get_gt diverged with the Iter impl prop_tree_matches_btreemap( @@ -1480,6 +1508,7 @@ fn tree_bug_24() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_25() { // postmortem: was not accounting for merges when traversing // the frag chain and a Del was encountered @@ -1493,6 +1522,7 @@ fn tree_bug_25() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_26() { // postmortem: prop_tree_matches_btreemap( @@ -1518,6 +1548,7 @@ fn tree_bug_26() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_27() { // postmortem: was not accounting for the fact that deletions reduce the // chances of being able to split successfully. @@ -1589,6 +1620,7 @@ fn tree_bug_27() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_28() { // postmortem: prop_tree_matches_btreemap( @@ -1613,6 +1645,7 @@ fn tree_bug_28() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_29() { // postmortem: tree merge and split thresholds caused an infinite // loop while performing updates @@ -1755,6 +1788,7 @@ fn tree_bug_29() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_30() { // postmortem: prop_tree_matches_btreemap( @@ -1795,6 +1829,7 @@ fn tree_bug_30() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_31() { // postmortem: prop_tree_matches_btreemap( @@ -1819,6 +1854,7 @@ fn tree_bug_31() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_32() { // postmortem: the MAX_IVEC that predecessor used in reverse // iteration was setting the first byte to 0 even though we @@ -1833,6 +1869,7 @@ fn tree_bug_32() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_33() { // postmortem: the split point was being incorrectly // calculated when using the simplified prefix technique. @@ -1852,6 +1889,7 @@ fn tree_bug_33() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_34() { // postmortem: a safety check was too aggressive when // finding predecessors using the new simplified prefix @@ -1873,6 +1911,7 @@ fn tree_bug_34() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_35() { // postmortem: prefix lengths were being incorrectly // handled on splits. @@ -1909,6 +1948,7 @@ fn tree_bug_35() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_36() { // postmortem: suffix truncation caused // regions to be permanently inaccessible @@ -1931,6 +1971,7 @@ fn tree_bug_36() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_37() { // postmortem: suffix truncation was so // aggressive that it would cut into @@ -1951,6 +1992,7 @@ fn tree_bug_37() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_38() { // postmortem: Free pages were not being initialized in the // pagecache properly. @@ -1972,6 +2014,7 @@ fn tree_bug_38() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_39() { // postmortem: for _ in 0..100 { @@ -2237,6 +2280,7 @@ fn tree_bug_39() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_40() { // postmortem: deletions of non-existant keys were // being persisted despite being unneccessary. @@ -2250,6 +2294,7 @@ fn tree_bug_40() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_41() { // postmortem: indexing of values during // iteration was incorrect. @@ -2270,6 +2315,7 @@ fn tree_bug_41() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_42() { // postmortem: during refactoring, accidentally // messed up the index selection for merge destinations. @@ -2291,6 +2337,7 @@ fn tree_bug_42() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_43() { // postmortem: when changing the PageState to always // include a base node, we did not account for this @@ -2316,6 +2363,7 @@ fn tree_bug_43() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_44() { // postmortem: off-by-one bug related to LSN recovery // where 1 was added to the index when the recovered @@ -2340,7 +2388,9 @@ fn tree_bug_44() { 0, )) } + #[test] +#[cfg_attr(miri, ignore)] fn tree_bug_45() { // postmortem: recovery was not properly accounting for // the possibility of a segment to be maxed out, similar diff --git a/tests/test_tree_failpoints.rs b/tests/test_tree_failpoints.rs index cb4004ff7..688a67550 100644 --- a/tests/test_tree_failpoints.rs +++ b/tests/test_tree_failpoints.rs @@ -518,6 +518,7 @@ fn quickcheck_tree_with_failpoints() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_01() { // postmortem 1: model did not account for proper reasons to fail to start assert!(prop_tree_crashes_nicely( @@ -527,6 +528,7 @@ fn failpoints_bug_01() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_02() { // postmortem 1: the system was assuming the happy path across failpoints assert!(prop_tree_crashes_nicely( @@ -541,6 +543,7 @@ fn failpoints_bug_02() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_03() { // postmortem 1: this was a regression that happened because we // chose to eat errors about advancing snapshots, which trigger @@ -554,6 +557,7 @@ fn failpoints_bug_03() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_04() { // postmortem 1: the test model was not properly accounting for // writes that may-or-may-not be present due to an error. @@ -570,6 +574,7 @@ fn failpoints_bug_04() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_05() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -593,6 +598,7 @@ fn failpoints_bug_05() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_06() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -614,6 +620,7 @@ fn failpoints_bug_06() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_07() { // postmortem 1: We were crashing because a Segment was // in the SegmentAccountant's to_clean Vec, but it had @@ -649,6 +656,7 @@ fn failpoints_bug_07() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_08() { // postmortem 1: we were assuming that deletes would fail if buffer writes // are disabled, but that's not true, because deletes might not cause any @@ -681,6 +689,7 @@ fn failpoints_bug_08() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_09() { // postmortem 1: recovery was not properly accounting for // ordering issues around allocation and freeing of pages. @@ -735,6 +744,7 @@ fn failpoints_bug_09() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_10() { // expected to iterate over 50 but got 49 instead // postmortem 1: @@ -946,6 +956,7 @@ fn failpoints_bug_10() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_11() { // dupe lsn detected // postmortem 1: @@ -986,6 +997,7 @@ fn failpoints_bug_11() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_12() { // postmortem 1: we were not sorting the recovery state, which // led to divergent state across recoveries. TODO wut @@ -1010,6 +1022,7 @@ fn failpoints_bug_12() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_13() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1053,6 +1066,7 @@ fn failpoints_bug_13() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_14() { // postmortem 1: improper bounds on splits caused a loop to happen assert!(prop_tree_crashes_nicely( @@ -1084,6 +1098,7 @@ fn failpoints_bug_14() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_15() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1093,6 +1108,7 @@ fn failpoints_bug_15() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_16() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1102,6 +1118,7 @@ fn failpoints_bug_16() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_17() { // postmortem 1: during recovery we were not properly // filtering replaced pages in segments by the source @@ -1138,6 +1155,7 @@ fn failpoints_bug_17() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_18() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1147,6 +1165,7 @@ fn failpoints_bug_18() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_19() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1190,6 +1209,7 @@ fn failpoints_bug_19() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_20() { // postmortem 1: failed to filter out segments with // uninitialized segment ID's when creating a segment @@ -1201,6 +1221,7 @@ fn failpoints_bug_20() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_21() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1275,6 +1296,7 @@ fn failpoints_bug_21() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_22() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1284,6 +1306,7 @@ fn failpoints_bug_22() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_23() { // postmortem 1: failed to handle allocation failures assert!(prop_tree_crashes_nicely( @@ -1299,6 +1322,7 @@ fn failpoints_bug_23() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_24() { // postmortem 1: was incorrectly setting global // errors, and they were being used-after-free @@ -1309,6 +1333,7 @@ fn failpoints_bug_24() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_25() { // postmortem 1: after removing segment trailers, we // no longer have the invariant that a write @@ -1371,6 +1396,7 @@ fn failpoints_bug_25() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_26() { // postmortem 1: after removing segment trailers, we // no longer handled maxed segment recovery properly @@ -1443,6 +1469,7 @@ fn failpoints_bug_26() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_27() { // postmortem 1: a segment is recovered as empty at recovery, // which prevented its lsn from being known, and when the SA @@ -1483,6 +1510,7 @@ fn failpoints_bug_27() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_28() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1519,6 +1547,7 @@ fn failpoints_bug_28() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_29() { // postmortem 1: the test model was turning uncertain entries // into certain entries even when there was an intervening crash @@ -1547,6 +1576,7 @@ fn failpoints_bug_29() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_30() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1562,6 +1592,7 @@ fn failpoints_bug_30() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_31() { // postmortem 1: apply_batch_inner drops a RecoveryGuard, which in turn // drops a Reservation, and Reservation's drop implementation flushes @@ -1580,6 +1611,7 @@ fn failpoints_bug_31() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_32() { // postmortem 1: for _ in 0..10 { @@ -1591,6 +1623,7 @@ fn failpoints_bug_32() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_33() { // postmortem 1: assert!(prop_tree_crashes_nicely( @@ -1660,6 +1693,7 @@ fn failpoints_bug_33() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_34() { // postmortem 1: the implementation of make_durable was not properly // exiting the function when local durability was detected @@ -1697,6 +1731,7 @@ fn failpoints_bug_34() { } #[test] +#[cfg_attr(miri, ignore)] fn failpoints_bug_35() { // postmortem 1: use BatchOp::*; From b1e35f8148e8e96fcdbc0bbbc55bad35086aafb7 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 21 Apr 2020 00:36:36 -0500 Subject: [PATCH 11/16] Add workaround to avoid dirfd() DirEntry::metadata() is built upon dirfd() and openat(), which aren't yet shimmed in Miri. Instead, get the PathBuf for the DirEntry and call std::fs::metadata() on that when using Miri. --- src/pagecache/mod.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/pagecache/mod.rs b/src/pagecache/mod.rs index 38bfd5def..117bf04b6 100644 --- a/src/pagecache/mod.rs +++ b/src/pagecache/mod.rs @@ -1383,7 +1383,16 @@ impl PageCache { // it's possible the blob file was removed lazily // in the background and no longer exists - size += blob_file.metadata().map(|m| m.len()).unwrap_or(0); + #[cfg(not(miri))] + { + size += blob_file.metadata().map(|m| m.len()).unwrap_or(0); + } + + // workaround to avoid missing `dirfd` shim + #[cfg(miri)] + { + size += std::fs::metadata(blob_file.path()).map(|m| m.len()).unwrap_or(0); + } } Ok(size) From b49c178100cd118b0be25724e796f647a782989a Mon Sep 17 00:00:00 2001 From: David Cook Date: Sun, 17 May 2020 17:31:49 -0500 Subject: [PATCH 12/16] Add an empirical correction factor to memory limit --- src/sys_limits.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/sys_limits.rs b/src/sys_limits.rs index 25ed97c43..fde6d7859 100644 --- a/src/sys_limits.rs +++ b/src/sys_limits.rs @@ -105,5 +105,16 @@ pub fn get_memory_limit() -> u64 { max = MAX_USIZE; } + #[cfg(miri)] + { + // Miri has a significant memory consumption overhead. During a small + // test run, a memory amplification of ~35x was observed. Certain + // memory overheads may increase asymptotically with longer test runs, + // such as the interpreter's dead_alloc_map. Memory overhead is + // dominated by stacked borrows tags; the asymptotic behavior of this + // overhead needs further investigation. + max /= 40; + } + max } From 107c573fa49081acded6c790ea20fa4aba5e31ff Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 18 May 2020 22:42:06 -0500 Subject: [PATCH 13/16] Add optional optimizations for Miri Early return from debug_delay() and type-punning construction of Histograms, to reduce the amount of Stacked Borrows overhead --- Cargo.toml | 1 + src/debug_delay.rs | 6 ++++++ src/histogram.rs | 27 ++++++++++++++++++++++++--- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index df38bd949..e4950ca54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ measure_allocs = [] pretty_backtrace = ["color-backtrace"] io_uring = ["rio"] docs = [] +miri_optimizations = [] [dependencies] crossbeam-epoch = "0.8.2" diff --git a/src/debug_delay.rs b/src/debug_delay.rs index b212ac927..058e2c689 100644 --- a/src/debug_delay.rs +++ b/src/debug_delay.rs @@ -27,6 +27,12 @@ pub fn debug_delay() { static LOCAL_DELAYS: std::cell::RefCell = std::cell::RefCell::new(0) ); + if cfg!(feature = "miri_optimizations") { + // Each interaction with LOCAL_DELAYS adds more stacked borrows + // tracking information, and Miri is single-threaded anyway. + return; + } + let global_delays = GLOBAL_DELAYS.fetch_add(1, Relaxed); let local_delays = LOCAL_DELAYS.with(|ld| { let mut ld = ld.borrow_mut(); diff --git a/src/histogram.rs b/src/histogram.rs index 44abb05a7..dcb67e53c 100644 --- a/src/histogram.rs +++ b/src/histogram.rs @@ -45,11 +45,32 @@ pub struct Histogram { } impl Default for Histogram { + #[allow(unsafe_code)] fn default() -> Histogram { - let mut vals = Vec::with_capacity(BUCKETS); - vals.resize_with(BUCKETS, Default::default); + #[cfg(not(feature = "miri_optimizations"))] + { + let mut vals = Vec::with_capacity(BUCKETS); + vals.resize_with(BUCKETS, Default::default); + + Histogram { vals, sum: AtomicUsize::new(0), count: AtomicUsize::new(0) } + } - Histogram { vals, sum: AtomicUsize::new(0), count: AtomicUsize::new(0) } + #[cfg(feature = "miri_optimizations")] + { + // Avoid calling Vec::resize_with with a large length because its + // internals cause stacked borrows tracking information to add an + // item for each element of the vector. + let mut vals = std::mem::ManuallyDrop::new(vec![0usize; BUCKETS]); + let ptr: *mut usize = vals.as_mut_ptr(); + let len = vals.len(); + let capacity = vals.capacity(); + + let vals: Vec = unsafe { + Vec::from_raw_parts(ptr as *mut AtomicUsize, len, capacity) + }; + + Histogram { vals, sum: AtomicUsize::new(0), count: AtomicUsize::new(0) } + } } } From 387d6f32b82b4bbe72849eea09680c88ff566362 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 18 May 2020 23:43:17 -0500 Subject: [PATCH 14/16] Clippy fix --- src/histogram.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram.rs b/src/histogram.rs index dcb67e53c..6af57139f 100644 --- a/src/histogram.rs +++ b/src/histogram.rs @@ -60,7 +60,7 @@ impl Default for Histogram { // Avoid calling Vec::resize_with with a large length because its // internals cause stacked borrows tracking information to add an // item for each element of the vector. - let mut vals = std::mem::ManuallyDrop::new(vec![0usize; BUCKETS]); + let mut vals = std::mem::ManuallyDrop::new(vec![0_usize; BUCKETS]); let ptr: *mut usize = vals.as_mut_ptr(); let len = vals.len(); let capacity = vals.capacity(); From 13e5efe826cb19f6e94b47880c90c8ef8d1e68e1 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 3 Jun 2020 19:45:21 -0500 Subject: [PATCH 15/16] Conditionally ignore more tests --- tests/test_tree.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_tree.rs b/tests/test_tree.rs index 68720f55a..106a8c6df 100644 --- a/tests/test_tree.rs +++ b/tests/test_tree.rs @@ -547,6 +547,7 @@ fn tree_subdir() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_small_keys_iterator() { let config = Config::new().temporary(true).flush_every_ms(None); let t = config.open().unwrap(); @@ -586,6 +587,7 @@ fn tree_small_keys_iterator() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_big_keys_iterator() { fn kv(i: usize) -> Vec { let k = [(i >> 16) as u8, (i >> 8) as u8, i as u8]; @@ -818,6 +820,7 @@ fn create_tree() { } #[test] +#[cfg_attr(miri, ignore)] fn tree_import_export() -> Result<()> { common::setup_logger(); From 6284ab292a27b99a1e10a83887ceabbafbe3d646 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 23 Jun 2020 08:46:12 -0500 Subject: [PATCH 16/16] Check conditional compilation w/ miri config in CI --- scripts/cross_compile.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/cross_compile.sh b/scripts/cross_compile.sh index 0ac8432cc..26012ea27 100755 --- a/scripts/cross_compile.sh +++ b/scripts/cross_compile.sh @@ -17,6 +17,8 @@ for target in $targets; do cargo check --target $target done +RUSTFLAGS="--cfg miri" cargo check + rustup toolchain install 1.39.0 cargo clean rm Cargo.lock