Skip to content

fix(services/fs): avoid initializing unused writers on abort - #8429

Merged
Xuanwo merged 3 commits into
apache:mainfrom
passionworkeer:fix/fs-lazy-abort-20261025
Oct 30, 2026
Merged

Xuanwo merged 3 commits into
apache:mainfrom
passionworkeer:fix/fs-lazy-abort-20261025

Conversation

@passionworkeer

@passionworkeer passionworkeer commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #8428.

Rationale for this change

Aborting a lazy Fs writer currently initializes its inner writer. Cancelling before the first filesystem write can therefore truncate an existing file or create a target and its parents. An unused conditional writer can also fail with ConditionNotMatch even though there is nothing to cancel.

What changes are included in this PR?

Inspect FsLazyWriter.inner directly during abort: delegate to an existing writer, or return success when it has not been initialized. Keep initialization in write and close. This preserves the existing Unsupported result when aborting an initialized non-atomic writer and makes no rollback promise after writes.

Add seven tests against real temporary filesystem roots, covering untouched and buffered existing files, missing target directories, conditional writes, atomic target directories, empty close, and initialized non-atomic abort.

Validation

Base 19ba33da9e46738fd4cad29c7ebfd5f3e5ed2cfc: 5 expected regression failures and 2 passing controls. Fix 546abff605152e5baa36bf6212fa33514c4b0219:

  • Fs service: 8 unit tests, 7 regressions, and 1 doctest passed; 1 existing doctest ignored.
  • Official filesystem behavior suite: 130 passed with ordinary writes; 130 passed with atomic_write_dir.
  • cargo clippy --locked -p opendal-service-fs --all-targets --all-features -- -D warnings passed.
  • Repository-wide python3 scripts/workspace.py cargo fmt -- --check passed.

Linux, Rust/Cargo 1.99.1. This verifies the affected filesystem service; no full workspace build, other storage service, or cross-platform CI result is claimed.

Follow-up 9058aa2c56d2f77dbf6310efb9d3559d97b1f641 normalizes four spaces in the test file's Apache license header to match the repository checker. It changes no executable code (git diff --ignore-all-space is empty). HawkEye 7.2.0, the Fs service tests (8 unit, 7 regression, 1 doctest passed; 1 ignored), and core formatting passed on this final commit.

Are there any user-facing changes?

Aborting an uninitialized Fs writer preserves existing file contents and avoids creating the target or its parents. Closing an unused writer and aborting an initialized non-atomic writer retain their existing behavior.

Breaking changes

AI Usage Statement

  • Harness: Codex desktop.
  • Model: the primary execution model identifier was not exposed to the agent; delegated investigation and independent review used gpt-6.1-sol.
  • Effort: primary and inherited subagent reasoning-effort settings were not exposed to the agent.
  • Role: source investigation, implementation, real filesystem reproduction and regression execution, and independent code review. The separate OCR reviewer could not execute because its provider configuration was missing. The human contributor reviewed the submission and patch and explicitly approved publication.

CI follow-up (2026-10-26)

The remaining failure is Bindings Lua CI / test: luarocks make requests the absent lua51 feature. The October 22 main-branch run, at 39f6bc7d91534b17e1c0182cb556aeab6cfd6742, fails with the same message before the Lua tests. bindings/lua/ and .github/workflows/ci_bindings_lua.yml are unchanged between that main commit and this PR head, and between the current PR base and head. This establishes a pre-existing Lua build configuration failure; no Lua change has been folded into the Fs fix. The failing check remains outstanding, and no full-CI pass or exemption is claimed. Codex assisted this source/log comparison; no new build was run for this diagnosis.

Copilot AI balanced review requested due to automatic review settings October 25, 2026 06:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions github-actions Bot added releases-note/fix The PR fixes a bug or has a title that begins with "fix" services/fs size:L This PR changes 100-499 lines, ignoring generated files. labels Oct 25, 2026
@Xuanwo
Xuanwo merged commit 13a4455 into apache:main Oct 30, 2026
158 of 159 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/fix The PR fixes a bug or has a title that begins with "fix" services/fs size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(services/fs): aborting a lazy writer can truncate an existing file

3 participants