opendal icon indicating copy to clipboard operation
opendal copied to clipboard

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

Open sundy-li opened this issue 1 year ago • 7 comments

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 avatar Jan 23 '24 14:01 sundy-li

Also affects hdfs

Xuanwo avatar Jan 23 '24 14:01 Xuanwo

@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

hoslo avatar Feb 03 '24 07:02 hoslo

@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<()>).

Xuanwo avatar Feb 03 '24 11:02 Xuanwo

@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?

hoslo avatar Feb 03 '24 11:02 hoslo

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

Welcome! Thanks in advance.

Xuanwo avatar Feb 03 '24 11:02 Xuanwo

@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?

tisonkun avatar Apr 01 '24 06:04 tisonkun

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 https://github.com/tokio-rs/tokio/pull/6330

Xuanwo avatar Apr 01 '24 08:04 Xuanwo