Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interrupted write causes corruption #243

Closed
tabokie opened this issue Jul 14, 2022 · 4 comments · Fixed by #245
Closed

Interrupted write causes corruption #243

tabokie opened this issue Jul 14, 2022 · 4 comments · Fixed by #245
Labels
bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@tabokie
Copy link
Member

tabokie commented Jul 14, 2022

At this line, we use write_all to append some bytes:

self.writer.write_all(buf)?;

If this write is interrupted, we directly bubble its error. But some portion of the data might already be written. In this case, the self.written is inconsistent with underlying writer's internal offset. A fractured write will remain as a phantom record.

@tabokie tabokie added bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Jul 14, 2022
@LykxSassinator
Copy link
Contributor

Maybe we need a new func to support a safe write here? Just like:

index 3f7628c..5f37581 100644
--- a/src/env/mod.rs
+++ b/src/env/mod.rs
@@ -55,4 +55,8 @@ pub trait WriteExt {
     fn truncate(&mut self, offset: usize) -> Result<()>;
     fn sync(&mut self) -> Result<()>;
     fn allocate(&mut self, offset: usize, size: usize) -> Result<()>;
+    fn write_all_safely(
+        &mut self,
+        buf: &mut [u8],
+    ) -> ::std::result::Result<usize, (usize, std::io::Error)>;
 }

@tabokie
Copy link
Member Author

tabokie commented Jul 15, 2022

No, simply reseek the writer if there's a failure. If that seek fails, panic.

@LykxSassinator
Copy link
Contributor

LykxSassinator commented Jul 15, 2022

Emm...I didn't get it.
reseek is just a op which resets the offset, and in our self-defined write, reseek and its followed operations in write is just like a redo op by continue triggered by Errno: EINTER.

https://github.com/LykxSassinator/raft-engine/blob/f3c268bb954f63f2809f84a940141db4419b2c44/src/env/default.rs#L116-L138

And I just wanna introduce a safe write by write_all_safely. If it failed, it would return the tuple, both containing the actual written size of bytes and the error details.

@tabokie
Copy link
Member Author

tabokie commented Jul 15, 2022

The purpose here is to make sure subsequent writes can correctly overwrite the failed partial write, so that LogFileWriter::written is always consistent with LogFile::offset, no phantom data is inserted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants