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

fs::Writer::poll_close can't be retried multiple times when error occurs #4058

Open
sundy-li opened this issue Jan 23, 2024 · 7 comments
Open
Assignees
Labels
bug Something isn't working services/fs services/hdfs

Comments

@sundy-li
Copy link
Contributor

https://github.com/apache/opendal/blob/3c4e2f1c42b88323ed13c4eb56b95e75b98b75a9/core/src/services/fs/writer.rs#L67C8-L91

    fn poll_close(&mut self, cx: &mut Context<'_>) -> Poll<Result<()>> {
        loop {
            if let Some(fut) = self.fut.as_mut() {
                let res = ready!(fut.poll_unpin(cx));
                self.fut = None;
                return Poll::Ready(res);
            }


            let mut f = self.f.take().expect("FsWriter must be initialized");
            ...
    }

If any error happens(eg: disk full), retry layer will retry to call poll_close function, but the self.f is taken, so panic happens.

@sundy-li sundy-li added the bug Something isn't working label Jan 23, 2024
@Xuanwo Xuanwo changed the title oio::Write::poll_close can't be retried multiple times when error occurs fs::Writer::poll_close can't be retried multiple times when error occurs Jan 23, 2024
@Xuanwo
Copy link
Member

Xuanwo commented Jan 23, 2024

Also affects hdfs

@Xuanwo Xuanwo self-assigned this Jan 25, 2024
@hoslo
Copy link
Contributor

hoslo commented Feb 3, 2024

@Xuanwo It seems to be running fine during the retry. Do you see any issues with this?
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b7bb0b4ad4ddc48cc08566694b41adb8

@Xuanwo
Copy link
Member

Xuanwo commented Feb 3, 2024

@Xuanwo It seems to be running fine during the retry. Do you see any issues with this? https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b7bb0b4ad4ddc48cc08566694b41adb8

Apologies for not updating you on the progress. The main issue is that poll_close takes F and doesn't return it when an error occurs. We could adjust the future to instead return a (F, Result<()>).

@hoslo
Copy link
Contributor

hoslo commented Feb 3, 2024

@Xuanwo重试期间似乎运行良好。您认为这有什么问题吗?https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b7bb0b4ad4ddc48cc08566694b41adb8

很抱歉没有向您通报进展情况。主要问题是发生错误时poll_close接受F但不返回它。我们可以调整 future 以返回(F, Result<()>).

My link above does this, can I create a new PR for this?

@Xuanwo
Copy link
Member

Xuanwo commented Feb 3, 2024

My link above does this, can I create a new PR for this?

Welcome! Thanks in advance.

@tisonkun
Copy link
Member

tisonkun commented Apr 1, 2024

@Xuanwo what is the progress of this issue?

I saw #4141 has been superseded by #4309, but perhaps the original issue is yet to resolved?

@Xuanwo
Copy link
Member

Xuanwo commented Apr 1, 2024

I saw #4141 has been superseded by #4309, but perhaps the original issue is yet to resolved?

Yes, I'm working with the tokio team on this tokio-rs/tokio#6330

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working services/fs services/hdfs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants