-
Notifications
You must be signed in to change notification settings - Fork 88
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
hide more file writing details from engine #269
Conversation
Signed-off-by: tabokie <[email protected]>
Signed-off-by: tabokie <[email protected]>
Signed-off-by: tabokie <[email protected]>
…901-parallel-write Signed-off-by: tabokie <[email protected]>
Signed-off-by: tabokie <[email protected]>
/cc @LykxSassinator |
Codecov ReportBase: 97.65% // Head: 97.66% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #269 +/- ##
==========================================
+ Coverage 97.65% 97.66% +0.01%
==========================================
Files 30 30
Lines 10645 10633 -12
==========================================
- Hits 10395 10385 -10
+ Misses 250 248 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: tabokie <[email protected]>
Signed-off-by: tabokie <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest is LGTM
@@ -401,15 +413,11 @@ impl<F: FileSystem> SinglePipe<F> { | |||
Ok(handle) | |||
} | |||
|
|||
fn maybe_sync(&self, force: bool) -> Result<()> { | |||
fn sync(&self) -> Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In current implementation of sync
, it still gets the mutex
for avoiding concurrent racing at the beginning.
According to #258 , will it be refactored to support concurrent accessing in later work ? i.e. replace mutex
with Rwlock
? or add an extra F::Handle
in SinglePipe
, making it be called concurrently ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Signed-off-by: tabokie <[email protected]>
Signed-off-by: tabokie <[email protected]>
Ping @Connor1996 , this one is blocking #261 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is a preparing work for #258.
Changes:
fsync
to file handle instead of writer, so that we can call it concurrently in the future. In doing so, we have to remove the sync offset tracking and deprecatebytes_per_sync
supportprepare_write
into pipe_log. Introduce a traitReactiveBytes
for this purpose.