From 395d530632a79e96d501b99390799f94e7488ac4 Mon Sep 17 00:00:00 2001
From: Yang Zhang <yang.zhang@pingcap.com>
Date: Tue, 28 Nov 2023 13:42:43 -0800
Subject: [PATCH] Add more details to rotate test

Signed-off-by: Yang Zhang <yang.zhang@pingcap.com>
---
 src/file_pipe_log/log_file.rs     |  2 +-
 src/file_pipe_log/pipe.rs         |  4 +---
 tests/failpoints/test_io_error.rs | 24 +++++++++++++++++++++++-
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/file_pipe_log/log_file.rs b/src/file_pipe_log/log_file.rs
index 98e11fa3..4341a75b 100644
--- a/src/file_pipe_log/log_file.rs
+++ b/src/file_pipe_log/log_file.rs
@@ -121,7 +121,7 @@ impl<F: FileSystem> LogFileWriter<F> {
         let _t = StopWatch::new(&*LOG_SYNC_DURATION_HISTOGRAM);
         // Panic if sync fails, in case of data loss.
         self.handle.sync().unwrap();
-        IoResult::Ok(())
+        Ok(())
     }
 
     #[inline]
diff --git a/src/file_pipe_log/pipe.rs b/src/file_pipe_log/pipe.rs
index 9b9debf6..8d0a4906 100644
--- a/src/file_pipe_log/pipe.rs
+++ b/src/file_pipe_log/pipe.rs
@@ -395,9 +395,7 @@ impl<F: FileSystem> SinglePipe<F> {
         let mut writable_file = self.writable_file.lock();
         let writer = &mut writable_file.writer;
         let _t = StopWatch::new(perf_context!(log_sync_duration));
-        if let Err(e) = writer.sync() {
-            return Err(Error::Io(e));
-        }
+        writer.sync().map_err(|e| Error::Io(e))?;
         Ok(())
     }
 
diff --git a/tests/failpoints/test_io_error.rs b/tests/failpoints/test_io_error.rs
index f0f734d0..88a50323 100644
--- a/tests/failpoints/test_io_error.rs
+++ b/tests/failpoints/test_io_error.rs
@@ -170,6 +170,7 @@ fn test_file_rotate_error() {
             .is_err());
         assert_eq!(engine.file_span(LogQueue::Append).1, 1);
     }
+    let num_files_before = std::fs::read_dir(&dir).unwrap().count();
     {
         // Fail to write header of new log file.
         let _f = FailGuard::new("log_file::write::err", "1*off->return");
@@ -177,6 +178,11 @@ fn test_file_rotate_error() {
             .write(&mut generate_batch(1, 4, 5, Some(&entry)), false)
             .is_err());
         assert_eq!(engine.file_span(LogQueue::Append).1, 1);
+        // Although the header is not written, the file is still created.
+        assert_eq!(
+            std::fs::read_dir(&dir).unwrap().count() - num_files_before,
+            1
+        );
     }
     {
         // Fail to sync new log file. The old log file is already sync-ed at this point.
@@ -186,14 +192,30 @@ fn test_file_rotate_error() {
         })
         .is_err());
         assert_eq!(engine.file_span(LogQueue::Append).1, 1);
+        // The file was created but without any content (header) should be
+        // recycled. And a new file should be created.
+        assert_eq!(
+            std::fs::read_dir(&dir).unwrap().count() - num_files_before,
+            1
+        );
     }
-
+    drop(engine);
+    let engine = Engine::open_with_file_system(cfg.clone(), fs.clone()).unwrap();
+    // Only one log file should be created after all the incidents.
+    assert_eq!(
+        std::fs::read_dir(&dir).unwrap().count() - num_files_before,
+        1
+    );
     // We can continue writing after the incidents.
     engine
         .write(&mut generate_batch(2, 1, 2, Some(&entry)), true)
         .unwrap();
     drop(engine);
     let engine = Engine::open_with_file_system(cfg, fs).unwrap();
+    assert_eq!(
+        std::fs::read_dir(&dir).unwrap().count() - num_files_before,
+        1
+    );
     assert_eq!(engine.first_index(1).unwrap(), 1);
     assert_eq!(engine.last_index(1).unwrap(), 4);
     assert_eq!(engine.first_index(2).unwrap(), 1);