Skip to content

feat(core): not translate to "interrupted" std IO error#7940

Merged
Xuanwo merged 4 commits into
apache:mainfrom
dentiny:hjiang/fix-io-error-prop
Jul 23, 2026
Merged

feat(core): not translate to "interrupted" std IO error#7940
Xuanwo merged 4 commits into
apache:mainfrom
dentiny:hjiang/fix-io-error-prop

Conversation

@dentiny

@dentiny dentiny commented Jul 23, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A

Rationale for this change

Hi team, I found when I'm using synchronous IO, it often retries more than I expected. See my regression test for details.

I think the root cause is

// Note that we don't track already initialized bytes here, but this is fine
// because we explicitly limit the read size
  let mut cursor = read_buf.unfilled();
  let result = loop {
    match r.read_buf(cursor.reborrow()) {
      Err(e) if e.is_interrupted() => continue,
        // Do not stop now in case of error: we might have received both data
        // and an error
        res => break res,
      }
};

I think it's not ideal because

  • Extra retries are attempted even for persistent errors, which we shouldn't retry upon
  • When retry layer is injected, extra retries are proceeded, even retry attempts have been exhausted
  • There's no retry limit control -- it just retries forever

What changes are included in this PR?

In this PR, I updated the std IO error from "interrupted" to "other", so it's directly propagated up; I checked OpenDAL errors, no errors should be mapped to "interrupted".
If retry is necessary, users could configure retry wrapper.

Are there any user-facing changes?

No

AI Usage Statement

AI helped me find out the source code.

@dentiny
dentiny requested a review from Xuanwo as a code owner July 23, 2026 06:00
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Jul 23, 2026
@dentiny dentiny changed the title fix(core): fix error propagation feat(core): not translate to "interrupted" std IO error Jul 23, 2026

@Xuanwo Xuanwo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thank you for fixing this

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 23, 2026
@Xuanwo
Xuanwo merged commit aab4bda into apache:main Jul 23, 2026
168 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants