Failure other than sync should be returned

Signed-off-by: v01dstar <[email protected]>
tikv · Nov 9, 2023 · 0d2924b · 0d2924b
1 parent 862fe0b
commit 0d2924b
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 25 deletions.
diff --git a/src/file_pipe_log/log_file.rs b/src/file_pipe_log/log_file.rs
@@ -32,7 +32,7 @@ pub(super) fn build_file_writer<F: FileSystem>(
     force_reset: bool,
 ) -> Result<LogFileWriter<F>> {
     let writer = system.new_writer(handle.clone())?;
-    Ok(LogFileWriter::open(handle, writer, format, force_reset).unwrap())
+    LogFileWriter::open(handle, writer, format, force_reset)
 }
 
 /// Append-only writer for log file. It also handles the file header write.

diff --git a/src/file_pipe_log/pipe.rs b/src/file_pipe_log/pipe.rs
@@ -250,8 +250,7 @@ impl<F: FileSystem> SinglePipe<F> {
 
         let (path_id, handle) = self
             .recycle_file(new_seq)
-            .unwrap_or_else(|| self.new_file(new_seq))
-            .unwrap();
+            .unwrap_or_else(|| self.new_file(new_seq))?;
         let f = File::<F> {
             seq: new_seq,
             handle: handle.into(),
@@ -272,7 +271,7 @@ impl<F: FileSystem> SinglePipe<F> {
         // File header must be persisted. This way we can recover gracefully if power
         // loss before a new entry is written.
         new_file.writer.sync();
-        self.sync_dir(path_id).unwrap();
+        self.sync_dir(path_id)?;
 
         **writable_file = new_file;
         let len = {

diff --git a/tests/failpoints/test_io_error.rs b/tests/failpoints/test_io_error.rs
@@ -165,19 +165,17 @@ fn test_file_rotate_error() {
     {
         // Fail to create new log file.
         let _f = FailGuard::new("default_fs::create::err", "return");
-        assert!(catch_unwind_silent(|| {
-            let _ = engine.write(&mut generate_batch(1, 4, 5, Some(&entry)), false);
-        })
-        .is_err());
+        assert!(engine
+            .write(&mut generate_batch(1, 4, 5, Some(&entry)), false)
+            .is_err());
         assert_eq!(engine.file_span(LogQueue::Append).1, 1);
     }
     {
         // Fail to write header of new log file.
         let _f = FailGuard::new("log_file::write::err", "1*off->return");
-        assert!(catch_unwind_silent(|| {
-            let _ = engine.write(&mut generate_batch(1, 4, 5, Some(&entry)), false);
-        })
-        .is_err());
+        assert!(engine
+            .write(&mut generate_batch(1, 4, 5, Some(&entry)), false)
+            .is_err());
         assert_eq!(engine.file_span(LogQueue::Append).1, 1);
     }
     {
@@ -527,20 +525,17 @@ fn test_no_space_write_error() {
         cfg.dir = dir.path().to_str().unwrap().to_owned();
         cfg.spill_dir = Some(spill_dir.path().to_str().unwrap().to_owned());
         {
-            // Case 1: `Write` is abnormal for no space left, Engine should panic at
+            // Case 1: `Write` is abnormal for no space left, Engine should fail at
             // `rotate`.
             let cfg_err = Config {
                 target_file_size: ReadableSize(1),
                 ..cfg.clone()
             };
             let engine = Engine::open(cfg_err).unwrap();
             let _f = FailGuard::new("log_fd::write::no_space_err", "return");
-            assert!(catch_unwind_silent(|| {
-                engine
-                    .write(&mut generate_batch(2, 11, 21, Some(&entry)), true)
-                    .unwrap_err();
-            })
-            .is_err());
+            assert!(engine
+                .write(&mut generate_batch(2, 11, 21, Some(&entry)), true)
+                .is_err());
             assert_eq!(
                 0,
                 engine
@@ -554,12 +549,9 @@ fn test_no_space_write_error() {
             let _f1 = FailGuard::new("log_fd::write::no_space_err", "2*return->off");
             let _f2 = FailGuard::new("file_pipe_log::force_choose_dir", "return");
             // The first write should fail, because all dirs run out of space for writing.
-            assert!(catch_unwind_silent(|| {
-                engine
-                    .write(&mut generate_batch(2, 11, 21, Some(&entry)), true)
-                    .unwrap_err();
-            })
-            .is_err());
+            assert!(engine
+                .write(&mut generate_batch(2, 11, 21, Some(&entry)), true)
+                .is_err());
             assert_eq!(
                 0,
                 engine