Failrs (#600)

* Single thread tests

* Isolating fail tests into a different binary

Cargo.toml

@@ -83,3 +83,19 @@ wasm-bindgen = ["uuid/wasm-bindgen"]
 
 [badges]
 travis-ci = { repository = "tantivy-search/tantivy" }
+
+[dev-dependencies.fail]
+features = ["failpoints"]
+
+
+# Following the "fail" crate best practises, we isolate
+# tests that define specific behavior in fail check points
+# in a different binary.
+#
+# We do that because, fail rely on a global definition of
+# failpoints behavior and hence, it is incompatible with
+# multithreading.
+[[test]]
+name = "failpoints"
+path = "tests/failpoints/mod.rs"
+required-features = ["fail/failpoints"]

appveyor.yml

@@ -18,5 +18,5 @@ install:
 build: false
 
 test_script:
-  - REM SET RUST_LOG=tantivy,test & cargo test --verbose --no-default-features --features mmap -- --test-threads 1
+  - REM SET RUST_LOG=tantivy,test & cargo test --verbose --no-default-features --features mmap
   - REM SET RUST_BACKTRACE=1 & cargo build --examples

run-tests.sh

@@ -1,2 +1,2 @@
 #!/bin/bash
-cargo test --features "failpoints"
+cargo test

src/directory/managed_directory.rs

@@ -135,28 +135,28 @@ impl ManagedDirectory {
                         files_to_delete.push(managed_path.clone());
                     }
                 }
+            } else {
+                error!("Failed to acquire lock for GC");
             }
         }
 
         let mut deleted_files = vec![];
-        {
-            for file_to_delete in files_to_delete {
-                match self.delete(&file_to_delete) {
-                    Ok(_) => {
-                        info!("Deleted {:?}", file_to_delete);
-                        deleted_files.push(file_to_delete);
-                    }
-                    Err(file_error) => {
-                        match file_error {
-                            DeleteError::FileDoesNotExist(_) => {
-                                deleted_files.push(file_to_delete);
-                            }
-                            DeleteError::IOError(_) => {
-                                if !cfg!(target_os = "windows") {
-                                    // On windows, delete is expected to fail if the file
-                                    // is mmapped.
-                                    error!("Failed to delete {:?}", file_to_delete);
-                                }
+        for file_to_delete in files_to_delete {
+            match self.delete(&file_to_delete) {
+                Ok(_) => {
+                    info!("Deleted {:?}", file_to_delete);
+                    deleted_files.push(file_to_delete);
+                }
+                Err(file_error) => {
+                    match file_error {
+                        DeleteError::FileDoesNotExist(_) => {
+                            deleted_files.push(file_to_delete);
+                        }
+                        DeleteError::IOError(_) => {
+                            if !cfg!(target_os = "windows") {
+                                // On windows, delete is expected to fail if the file
+                                // is mmapped.
+                                error!("Failed to delete {:?}", file_to_delete);
                             }
                         }
                     }
@@ -171,11 +171,9 @@ impl ManagedDirectory {
                 .meta_informations
                 .write()
                 .expect("Managed directory wlock poisoned (2).");
-            {
-                let managed_paths_write = &mut meta_informations_wlock.managed_paths;
-                for delete_file in &deleted_files {
-                    managed_paths_write.remove(delete_file);
-                }
+            let managed_paths_write = &mut meta_informations_wlock.managed_paths;
+            for delete_file in &deleted_files {
+                managed_paths_write.remove(delete_file);
             }
             if save_managed_paths(self.directory.as_mut(), &meta_informations_wlock).is_err() {
                 error!("Failed to save the list of managed files.");

src/directory/mmap_directory.rs

@@ -417,7 +417,6 @@ impl Directory for MmapDirectory {
     /// Any entry associated to the path in the mmap will be
     /// removed before the file is deleted.
     fn delete(&self, path: &Path) -> result::Result<(), DeleteError> {
-        debug!("Deleting file {:?}", path);
         let full_path = self.resolve_path(path);
         match fs::remove_file(&full_path) {
             Ok(_) => self

src/directory/mod.rs

@@ -29,7 +29,7 @@ use std::io::{BufWriter, Write};
 #[cfg(feature = "mmap")]
 pub use self::mmap_directory::MmapDirectory;
 
-pub(crate) use self::managed_directory::ManagedDirectory;
+pub use self::managed_directory::ManagedDirectory;
 
 /// Write object for Directory.
 ///

src/directory/ram_directory.rs

@@ -145,6 +145,11 @@ impl Directory for RAMDirectory {
     }
 
     fn delete(&self, path: &Path) -> result::Result<(), DeleteError> {
+        fail_point!("RAMDirectory::delete", |_| {
+            use crate::directory::error::IOError;
+            let io_error = IOError::from(io::Error::from(io::ErrorKind::Other));
+            Err(DeleteError::from(io_error))
+        });
         self.fs.write().unwrap().delete(path)
     }
 

src/indexer/index_writer.rs

@@ -761,6 +761,7 @@ mod tests {
     use crate::Index;
     use crate::ReloadPolicy;
     use crate::Term;
+    use fail;
 
     #[test]
     fn test_operations_group() {
@@ -933,7 +934,6 @@ mod tests {
 
     #[test]
     fn test_with_merges() {
-        let _guard = fail::FailScenario::setup();
         let mut schema_builder = schema::Schema::builder();
         let text_field = schema_builder.add_text_field("text", schema::TEXT);
         let index = Index::create_in_ram(schema_builder.build());
@@ -1044,32 +1044,6 @@ mod tests {
         assert_eq!(num_docs_containing("b"), 100);
     }
 
-    #[cfg(feature = "failpoints")]
-    #[test]
-    fn test_write_commit_fails() {
-        let _guard = fail::FailScenario::setup();
-        let mut schema_builder = schema::Schema::builder();
-        let text_field = schema_builder.add_text_field("text", schema::TEXT);
-        let index = Index::create_in_ram(schema_builder.build());
-
-        let mut index_writer = index.writer_with_num_threads(1, 3_000_000).unwrap();
-        for _ in 0..100 {
-            index_writer.add_document(doc!(text_field => "a"));
-        }
-        index_writer.commit().unwrap();
-        fail::cfg("RAMDirectory::atomic_write", "return(error_write_failed)").unwrap();
-        for _ in 0..100 {
-            index_writer.add_document(doc!(text_field => "b"));
-        }
-        assert!(index_writer.commit().is_err());
-        let num_docs_containing = |s: &str| {
-            let term_a = Term::from_field_text(text_field, s);
-            index.reader().unwrap().searcher().doc_freq(&term_a)
-        };
-        assert_eq!(num_docs_containing("a"), 100);
-        assert_eq!(num_docs_containing("b"), 0);
-    }
-
     #[test]
     fn test_add_then_delete_all_documents() {
         let mut schema_builder = schema::Schema::builder();

tests/failpoints/mod.rs

@@ -0,0 +1,67 @@
+use fail;
+use std::io::Write;
+use std::path::Path;
+use tantivy::directory::{Directory, ManagedDirectory, RAMDirectory};
+use tantivy::doc;
+use tantivy::schema::{Schema, TEXT};
+use tantivy::{Index, Term};
+
+#[test]
+fn test_failpoints_managed_directory_gc_if_delete_fails() {
+    let scenario = fail::FailScenario::setup();
+
+    let test_path: &'static Path = Path::new("some_path_for_test");
+
+    let ram_directory = RAMDirectory::create();
+    let mut managed_directory = ManagedDirectory::wrap(ram_directory).unwrap();
+    managed_directory
+        .open_write(test_path)
+        .unwrap()
+        .flush()
+        .unwrap();
+    assert!(managed_directory.exists(test_path));
+    // triggering gc and setting the delete operation to fail.
+    //
+    // We are checking that the gc operation is not removing the
+    // file from managed.json to ensure that the file will be removed
+    // in the next gc.
+    //
+    // The initial 1*off is there to allow for the removal of the
+    // lock file.
+    fail::cfg("RAMDirectory::delete", "1*off->1*return").unwrap();
+    managed_directory.garbage_collect(Default::default);
+    assert!(managed_directory.exists(test_path));
+
+    // running the gc a second time should remove the file.
+    managed_directory.garbage_collect(Default::default);
+    assert!(
+        !managed_directory.exists(test_path),
+        "The file should have been deleted"
+    );
+}
+
+#[test]
+fn test_write_commit_fails() {
+    let _fail_scenario_guard = fail::FailScenario::setup();
+    let mut schema_builder = Schema::builder();
+    let text_field = schema_builder.add_text_field("text", TEXT);
+    let index = Index::create_in_ram(schema_builder.build());
+
+    let mut index_writer = index.writer_with_num_threads(1, 3_000_000).unwrap();
+    for _ in 0..100 {
+        index_writer.add_document(doc!(text_field => "a"));
+    }
+    index_writer.commit().unwrap();
+    fail::cfg("RAMDirectory::atomic_write", "return(error_write_failed)").unwrap();
+    for _ in 0..100 {
+        index_writer.add_document(doc!(text_field => "b"));
+    }
+    assert!(index_writer.commit().is_err());
+
+    let num_docs_containing = |s: &str| {
+        let term_a = Term::from_field_text(text_field, s);
+        index.reader().unwrap().searcher().doc_freq(&term_a)
+    };
+    assert_eq!(num_docs_containing("a"), 100);
+    assert_eq!(num_docs_containing("b"), 0);
+}

tests/mod.rs

@@ -0,0 +1 @@
+mod failpoints;