[RuixiangMa]

Description

Fixes the existing DfsOnlyBatchFailureOnlyRevokesFailedKey test, which failed because its injected DFS write failure persisted beyond the intended operation.

Root cause

The test adapter failed every write from the configured call onward. The DFS backend retries a failed POSIX write once after reopening the file descriptor, so the injected failure affected both the original write and its internal retry. It also caused the test’s later, independent Put retry to fail, contrary to the scenario the test intends to verify.

Changes

• Preserve the persistent failure behavior so the batch write and its internal reopen retry both fail as intended.
• Add ClearWriteFailure() and call it immediately after BatchPut returns, before checking that a later Put can reuse the key.
• Keep the change limited to test code; no production behavior changes.

Testing

• DfsOnlyBatchFailureOnlyRevokesFailedKey passes.
• BatchDfsFailureOnlyRevokesFailedKey passes.
• DfsMetricsCountWriteFailureSeparatelyFromOps passes.

Suggested Merge Priority

• P0 — Urgent: Critical security vulnerability, data loss/corruption, or widespread production outage requiring immediate action
• P1 — High: Significant correctness or stability issue in existing functionality, or a regression blocking users without a practical workaround
• P2 — Normal: Routine bug fix, validated performance/compatibility improvement, or new feature for a confirmed use case
• P3 — Low: Experimental feature, optional enhancement, refactoring, or cleanup with no urgent user impact

Module

• Transfer Engine (mooncake-transfer-engine)
• Mooncake Store (mooncake-store)
• Mooncake Conductor (mooncake-conductor)
• Reshard (mooncake-reshard)
• Mooncake EP (mooncake-ep)
• Mooncake PG (mooncake-pg)
• Integration (mooncake-integration)
• P2P Store (mooncake-p2p-store)
• Python Wheel (mooncake-wheel)
• Common (mooncake-common)
• Mooncake RL (mooncake-rl)
• CI/CD
• Docs
• Other

Type of Change

• Bug fix
• New feature
• Refactor
• Breaking change
• Documentation update
• Performance improvement
• Other

How Has This Been Tested?

Test commands:

# Example: bash scripts/run_ci_test.sh

Test results:

• Unit tests pass
• Integration tests pass (if applicable)
• Manual testing done (describe below)

Checklist

• I have performed a self-review of my own code
• I have formatted my code using ./scripts/code_format.sh
• I have run pre-commit on the files changed in this PR and all hooks pass
• I have updated the documentation (if applicable)
• I have added tests to prove my changes are effective
• For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

• No AI tools were used
• AI tools were used (specify below)

[he-yufeng]

Verified this in a Linux container (ubuntu 22.04, mooncake-store tests): DfsOnlyBatchFailureOnlyRevokesFailedKey passes with the change, and the full dfs_sync_client_test suite goes 16 passed / 2 environment-skipped (the two checksum tests skip for lack of a DFS endpoint here, same as before).

The disarm placement and ClearWriteFailure helper are correct: fail_write_call_ = -1 defeats the fail > 0 guard, and clearing right after the failing batch leaves the retry leg modeling a cleared transient fault, which is the scenario under test.

One nit: the deleted comment block above the fail > 0 guard was load-bearing documentation — it explains why the injector is persistent at all (a one-shot failure would be masked by the backend's reopen-retry, so only a persistent fault surfaces to the caller). With it gone, the next person touching this adapter has to rediscover that. Keeping it (or moving it next to ClearWriteFailure) would make the fix complete.

For disclosure: I independently opened #4785 with the same fix last night before noticing this one (identical mechanism, existing FailWriteCall(-1) instead of a new helper). Closing mine in favor of this earlier one.

[fcczzz]

LGTM, the fix correctly clears the injected fault before the independent retry.

One non-blocking suggestion: please retain the existing comment in WriteAt explaining why failures must persist across the backend’s reopen retry. That context is still relevant and helps prevent accidentally changing the injector back to a single-call failure.