update tests

src/mock.rs

@@ -20,18 +20,9 @@ impl std::ops::Deref for MockIndex {
 }
 
 impl MockIndex {
-    /// Used for tests only
-    pub fn insert_indirection(
-        &self,
-        key: &[u8],
-        value: ValueHandle,
-        size: u32,
-    ) -> std::io::Result<()> {
-        self.write()
-            .expect("lock is poisoned")
-            .insert(key.into(), (value, size));
-
-        Ok(())
+    /// Remove item
+    pub fn remove(&self, key: &[u8]) {
+        self.0.write().expect("lock is poisoned").remove(key);
     }
 }
 
@@ -61,7 +52,11 @@ impl IndexWriter for MockIndexWriter {
         value: ValueHandle,
         size: u32,
     ) -> std::io::Result<()> {
-        self.0.insert_indirection(key, value, size)
+        self.0
+            .write()
+            .expect("lock is poisoned")
+            .insert(key.into(), (value, size));
+        Ok(())
     }
 
     fn finish(&mut self) -> std::io::Result<()> {

tests/rollover_index_fail_finish.rs

@@ -1,59 +1,16 @@
-use std::{
-    collections::BTreeMap,
-    sync::{Arc, RwLock},
-};
-use value_log::{Config, IndexReader, IndexWriter, ValueHandle, ValueLog};
+use test_log::test;
+use value_log::{Config, IndexWriter, MockIndex, MockIndexWriter, ValueHandle, ValueLog};
 
-type DebugIndexInner = RwLock<BTreeMap<Arc<[u8]>, (ValueHandle, u32)>>;
-
-#[derive(Clone, Default)]
-pub struct DebugIndex(Arc<DebugIndexInner>);
-
-impl std::ops::Deref for DebugIndex {
-    type Target = DebugIndexInner;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl DebugIndex {
-    fn insert_indirection(&self, key: &[u8], value: ValueHandle, size: u32) -> std::io::Result<()> {
-        self.write()
-            .expect("lock is poisoned")
-            .insert(key.into(), (value, size));
-
-        Ok(())
-    }
-}
-
-impl IndexReader for DebugIndex {
-    fn get(&self, key: &[u8]) -> std::io::Result<Option<ValueHandle>> {
-        Ok(self
-            .read()
-            .expect("lock is poisoned")
-            .get(key)
-            .map(|(handle, _)| handle)
-            .cloned())
-    }
-}
-
-/// Used for tests only
 #[allow(clippy::module_name_repetitions)]
-pub struct DebugIndexWriter(DebugIndex);
+pub struct DebugIndexWriter;
 
 impl IndexWriter for DebugIndexWriter {
     fn insert_direct(&mut self, _: &[u8], _: &[u8]) -> std::io::Result<()> {
         Ok(())
     }
 
-    fn insert_indirect(
-        &mut self,
-        key: &[u8],
-        value: ValueHandle,
-        size: u32,
-    ) -> std::io::Result<()> {
-        self.0.insert_indirection(key, value, size)
+    fn insert_indirect(&mut self, _: &[u8], _: ValueHandle, _: u32) -> std::io::Result<()> {
+        Ok(())
     }
 
     fn finish(&mut self) -> std::io::Result<()> {
@@ -66,14 +23,14 @@ fn rollover_index_fail_finish() -> value_log::Result<()> {
     let folder = tempfile::tempdir()?;
     let vl_path = folder.path();
 
-    let index = DebugIndex(Arc::new(RwLock::new(BTreeMap::default())));
+    let index = MockIndex::default();
 
     let value_log = ValueLog::open(vl_path, Config::default())?;
 
     let items = ["a", "b", "c", "d", "e"];
 
     {
-        let index_writer = DebugIndexWriter(index.clone());
+        let index_writer = MockIndexWriter(index.clone());
         let mut writer = value_log.get_writer(index_writer)?;
 
         for key in &items {
@@ -83,22 +40,30 @@ fn rollover_index_fail_finish() -> value_log::Result<()> {
             writer.write(key.as_bytes(), value)?;
         }
 
-        // NOTE: Should return error because index fails
-        assert!(value_log.register_writer(writer).is_err());
+        value_log.register_writer(writer)?;
     }
 
     assert_eq!(value_log.manifest.list_segment_ids(), [0]);
-
     value_log.scan_for_stats(index.read().unwrap().values().cloned().map(Ok))?;
+    assert_eq!(1.0, value_log.space_amp());
+
+    index.remove(b"a");
+    index.remove(b"b");
+    index.remove(b"c");
+    index.remove(b"d");
 
+    value_log.scan_for_stats(index.read().unwrap().values().cloned().map(Ok))?;
     assert_eq!(
         value_log.manifest.get_segment(0).unwrap().stale_ratio(),
-        0.0
+        0.8
     );
+    assert!(value_log.space_amp() > 1.0);
 
-    let result = value_log.rollover(&[0], &index, DebugIndexWriter(index.clone()));
+    let result = value_log.rollover(&[0], &index, DebugIndexWriter);
     assert!(result.is_err());
 
+    // NOTE: Segment 1's value handles were not committed to index, so it's not referenced at all
+    // We get no data loss, the segment is just left dangling and can be removed
     assert_eq!(
         {
             let mut ids = value_log.manifest.list_segment_ids();
@@ -109,15 +74,22 @@ fn rollover_index_fail_finish() -> value_log::Result<()> {
     );
 
     value_log.scan_for_stats(index.read().unwrap().values().cloned().map(Ok))?;
-
     assert_eq!(
         value_log.manifest.get_segment(0).unwrap().stale_ratio(),
-        1.0
+        0.8
     );
+    assert!(value_log.space_amp() > 1.0);
 
     value_log.drop_stale_segments()?;
+    assert_eq!(value_log.manifest.list_segment_ids(), [0]);
+
+    index.remove(b"e");
 
-    assert_eq!(value_log.manifest.list_segment_ids(), [1]);
+    // NOTE: Now all values are stale, and everything can be dropped
+    value_log.scan_for_stats(index.read().unwrap().values().cloned().map(Ok))?;
+    value_log.drop_stale_segments()?;
+    assert_eq!(value_log.manifest.list_segment_ids(), []);
+    assert_eq!(0.0, value_log.space_amp());
 
     Ok(())
 }