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

(Question) How to sync underlying file correctly? #317

Open
adsick opened this issue Dec 3, 2024 · 5 comments
Open

(Question) How to sync underlying file correctly? #317

adsick opened this issue Dec 3, 2024 · 5 comments

Comments

@adsick
Copy link

adsick commented Dec 3, 2024

I have a GzipEncoder<File> and I need to finalize it.
I'd like to call sync_all() on it. But before I do that I think I need to finalize encoding first. As far as I understand it is done by calling .shutdown() on the encoder. But the problem is that it will also call .shutdown() on the underlying writer's .shutdown() (link)
I believe it's incorrect to call anything that potentially writes to a file after it was shutdown, so I'm in an egg vs chicken kind of situation right here.

Sorry for posting this as an issue, I just don't see any Discussions/forum to ask questions.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 3, 2024

You can call poll_flush to flush the data, then sync_all to sync the data and metadata before shutting it down.

@adsick
Copy link
Author

adsick commented Dec 4, 2024

Either I'm missing something or what you're proposing is incorrect.
In order to sync the file properly everything has to be written to it before calling sync_all - it means that encoder needs to be finished (not just flushed).

From what I see in public API the only way to finish encoding is to call .shutdown(), but it will also call shutdown on the underlying file (which might be a problem).

For files specifically this might not be a problem, because tokio::fs::File::shutdown is essentially the same as flush link.

It just seems like the API forces to break shutdown contract "do not read/write after shutdown".

@Nemo157
Copy link
Member

Nemo157 commented Dec 4, 2024

You could have an intermediate wrapper that doesn't forward shutdown to the underlying file, just flushes it and reports that it has been shutdown.

GzipEncoder<NoShutdown<File>>

@adsick
Copy link
Author

adsick commented Dec 4, 2024

Nice, this seems like an actual solution.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 4, 2024

Or you can change it to sync_all on shutdown

struct SyncOnShutdownFile {
    file: File,
    // JoinHandle is fused internally
    sync_all: Option<JoinHandle<io::Result<()>>>,
}

impl AsyncWrite {
    fn poll_shutdown(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<io::Result<()>> {
        let Self { file, sync_all } = Pin::into_inner(self);
        ready!(file.poll_flush(cx));

        if sync_all.is_none() {
            let file = file.try_clone()?.into_std();
            // currently tokio internally spawns, if io-uring is used then it's a different story
            *sync_all = tokio::task::spawn_blocking(|| f.sync_all());
        }
        ready!(Pin::new(sync_all.as_mut().unwrap())).map_err(io::Error::from)??;

        file.poll_shutdown(cx)
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants