Skip to content

Commit

Permalink
Address comments, fix test cases
Browse files Browse the repository at this point in the history
Signed-off-by: v01dstar <[email protected]>
  • Loading branch information
v01dstar committed Nov 7, 2023
1 parent 9b14b92 commit b0f6b3f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 18 deletions.
7 changes: 7 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,10 @@ pub(crate) fn is_no_space_err(e: &IoError) -> bool {
// `ErrorKind::StorageFull` is stable.
format!("{e}").contains("nospace")
}

/// Check whether the given error is a nospace error.
pub(crate) fn is_io_no_space_err(e: &Error) -> bool {
// TODO: make the following judgement more elegant when the error type
// `ErrorKind::StorageFull` is stable.
format!("{e}").contains("nospace")
}
14 changes: 8 additions & 6 deletions src/file_pipe_log/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use parking_lot::{Mutex, MutexGuard, RwLock};

use crate::config::Config;
use crate::env::{FileSystem, Permission};
use crate::errors::is_no_space_err;
use crate::errors::{is_io_no_space_err, is_no_space_err};
use crate::event_listener::EventListener;
use crate::metrics::*;
use crate::pipe_log::{
Expand Down Expand Up @@ -323,12 +323,12 @@ impl<F: FileSystem> SinglePipe<F> {
if writable_file.writer.offset() >= self.target_file_size {
if let Err(e) = self.rotate_imp(&mut writable_file) {
// As the disk is already full and is no valid and extra directory to flush data, it can safely return the `no space err` to users.
if is_no_space_err(&e) && self.paths.len() == 1 {
if is_io_no_space_err(&e) {
return Err(e);
}
panic!(
"error when rotate [{:?}:{}]: {e}",
self.queue, writable_file.seq,
"error when rotate [{:?}:{}]: {e}",
self.queue, writable_file.seq,
);
}
}
Expand Down Expand Up @@ -376,8 +376,10 @@ impl<F: FileSystem> SinglePipe<F> {
// - [3] Both main-dir and spill-dir have several recycled logs.
// But as `bytes.len()` is always smaller than `target_file_size` in common
// cases, this issue will be ignored temprorarily.
if let Err(e) = self.rotate_imp(&mut writable_file) && is_no_space_err(&e) {
return Err(e);
if let Err(e) = self.rotate_imp(&mut writable_file) {
if is_io_no_space_err(&e) {
return Err(e);
}
}
// If there still exists free space for this record, rotate the file
// and return a special TryAgain Err (for retry) to the caller.
Expand Down
18 changes: 6 additions & 12 deletions tests/failpoints/test_io_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,12 +535,9 @@ fn test_no_space_write_error() {
};
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
Expand All @@ -554,12 +551,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
Expand Down

0 comments on commit b0f6b3f

Please sign in to comment.