Skip to content

vdev_rebuild: account for failed repairs before retiring DTLs - #19121

Closed
Zaczero wants to merge 5 commits into
openzfs:masterfrom
Zaczero:fix/rebuild-error-accounting
Closed

Zaczero wants to merge 5 commits into
openzfs:masterfrom
Zaczero:fix/rebuild-error-accounting

Conversation

@Zaczero

@Zaczero Zaczero commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Motivation and Context

A sequential rebuild can report completion despite failed destination writes or dRAID rows it could not reconstruct. Too few readable columns can even cause unreconstructed contents to be written. The replacement may then detach the original while it still holds the only valid copy.

This affects sequential mirror and dRAID rebuilds from 2.0.0. Attach or replace without -s to use healing instead, including manual replacement onto a distributed spare.

Description

Count failed destination writes and refuse dRAID rows that cannot reconstruct a column requiring repair. A replacing or spare column successfully read from a complete child has already copied that segment to its new child, so damage elsewhere does not make it an uncopied repair.

This requires stable bounds (#19179), retirement based on the rebuild's result (#19195), and synchronization with in-place rewrites (#19198). Without the rewrite fix, healthy rows can appear contradictory.

How Has This Been Tested?

  • rebuild_write_errors covers failed, failfast and offline destinations; recovery must preserve the data after import.
  • rebuild_reset_errors verifies that a restart reports the old errors and starts a fresh count.
  • rebuild_draid_errors covers insufficient columns and contradictory parity; rebuild_draid verifies that healthy distributed-spare reconstruction still completes.

The regressions passed in a Linux debug guest alongside the retirement, source-selection and healing fixes.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied. (Selected coverage described above.)
  • All commit messages are properly formatted and contain Signed-off-by.

@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch 6 times, most recently from f412d6d to 1738874 Compare September 23, 2026 06:37
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch from 1738874 to 5967693 Compare September 23, 2026 11:15
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch from 5967693 to 2a93937 Compare September 23, 2026 15:54
@Zaczero Zaczero changed the title vdev_rebuild: preserve repair bounds and account for write failures vdev_rebuild: account for failed repairs before retiring DTLs Sep 23, 2026
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch from 2a93937 to 7cf4748 Compare September 26, 2026 08:14
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch 2 times, most recently from 2cd8643 to 8cd975d Compare September 27, 2026 18:28
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch 3 times, most recently from aab349a to 8bd93a5 Compare October 4, 2026 12:14
A failed rebuild write adds the rebuild's saved interval of missing
txgs to the device's DTL. The saved interval's minimum is exclusive, but
the failure is recorded from that minimum, one txg below the DTL. At
every metaslab the cancellation check then recalculates the interval in
place from the DTL, so the minimum moves one txg lower with each
metaslab that has failed writes. Once the DTL starts at txg 0, the next
calculation underflows and the rebuild records an invalid range.

The same recalculation can raise the upper bound after later ordinary
writes are missed. Completion can then retire those later missing txgs
without establishing that the rebuild covered them.

Make cancellation checks read-only, and convert the saved exclusive
minimum to the inclusive range-tree start without changing the exclusive
upper bound. Document both bounds in the persistent rebuild structure;
its layout is unchanged. Initiation and a full rebuild reset remain the
places that capture the interval. A DTL which already starts at txg 0,
as a rebuild could leave it, has no lower bound rather than one which
wraps around. This does not reconstruct bounds already changed by older
code.

Cover a failed target write after rebuild I/O has been persisted, and
later missing ordinary writes on mirrors and dRAID. Verify that saved
bounds survive export/import and completion, that completion retains
the later missing txg, and that healing completes recovery without the
original device. Exercise zero-start DTL queries through libzpool.

Signed-off-by: Kamil Monicz <kamil@monicz.dev>
A rebuild reconstructs the missing ranges of the devices it was started
for. When it completed cleanly, DTL reassessment also retired the DTL of
every other leaf in the top-level vdev that was not faulted or offline.
A clean rebuild of one device therefore discarded another device's
record of missing writes. Its stale blocks were then trusted until a
scrub or a checksum failure found them, and were lost if the remaining
copy failed first.

When the rebuild did not complete cleanly, reassessment fell through to
the state of the last healing or verification scan and retired the DTLs
if that scan had no errors. A rebuild whose reads failed therefore still
retired the new device's missing ranges, and a replacement detached its
original. The recovery override also erased the rebuild's error count,
leaving no record of the failures it overrode.

A rebuild repairs only the devices it was started for. Never retire the
DTL of another device, and retire the rebuilt devices' DTLs only when
the rebuild itself completed without errors or the override is set. A
device that is unwritable at completion also keeps its DTL, since a
failed probe can set cant_write before the vdev state changes. Keep the
rebuild's error count under the override.

Cover a writable device with missing writes beside a clean rebuild: its
DTL remains until a healing pass completes. After a clean scrub, rebuild
while every copy fails its reads: the new device must keep its DTL
until healing completes the replacement, and with the override the
rebuild must still report its errors.

Signed-off-by: Kamil Monicz <kamil@monicz.dev>
Before a sequential rebuild reads a metaslab, it disables allocations
from it and waits for pending allocations to sync. Later passes of the
syncing txg can still rewrite blocks born in that txg in place: a
rewrite allocates nothing, and the first pass has already moved their
allocations to the space map. A rebuild read which overlaps such a
rewrite sees some columns before it and some after. dRAID then finds
the row's parity inconsistent and withholds its repair, and a read
which completes before the rewrite can write the old block over the new
one on the device being rebuilt.

Wait for the txg that is syncing when the metaslab is disabled. No later
txg can allocate from it, so none can rewrite its blocks in place.

Signed-off-by: Kamil Monicz <kamil@monicz.dev>
A successful rebuild read does not imply that its repair writes
succeeded. Vdev children suppress ordinary error propagation, and a
replacing mirror may report success when only its old child accepted a
write. The rebuild could then retire missing ranges and let the original
device be detached while its replacement is incomplete.

Report failed writes through ZIO_POST_REBUILD_ERROR. The existing
io_post propagation carries this result through intermediate vdevs even
with DONT_PROPAGATE, and the rebuild's logical completion callback
counts a failed segment once. The report is made before the checks
which leave speculative and failfast errors out of DTL accounting, so
every failed repair write counts.

dRAID writes to unavailable children to maintain their DTLs, so failures
on such children do not count unless the child is being rebuilt. A
device being rebuilt always counts, even if it becomes writable again
before the rebuild completes.

Record errors in completion and reset history, and reset the count only
when restarting from offset zero, not when resuming a saved rebuild.

Cover failed writes, including failfast failures which are never
retried, with and without the default verification scrub, a new device
that is offline during its rebuild, a rebuild restarted by attaching
another device after failed writes, speculative dRAID repairs, and a
healthy distributed-spare rebuild. Require rebuild-local errors, that
the rebuild does not retire failed repairs before a healing pass
completes, and cold data after explicit healing.

No pool feature or on-disk structure layout changes are required.

Signed-off-by: Kamil Monicz <kamil@monicz.dev>
Sequential reconstruction has no checksum. A row with fewer readable
columns than it needs still passes verification, and its unreconstructed
contents could be used to repair other children. A row whose parity
contradicts its data is left unrepaired. Either way, the rebuild
completes as if the row had been repaired.

Report such a row through ZIO_POST_REBUILD_ERROR when a column it would
repair was not read. A spare or replacing column that was read from a
complete child has already been copied to its rebuilding child by that
mirror, so a row which contradicts damage elsewhere leaves nothing
incomplete. An unavailable column that the rebuild does not target also
counts; its row is already beyond its parity. The logical read error and
the verified-row assertions are unchanged.

Cover dRAID rebuilds whose rows have too few readable columns, and whose
parity contradicts damaged data returned by one child while the new
device's column is reconstructed. The existing damaged-device tests
cover contradictions that the mirror has already copied.

Signed-off-by: Kamil Monicz <kamil@monicz.dev>
@Zaczero
Zaczero force-pushed the fix/rebuild-error-accounting branch from 8bd93a5 to e9b8216 Compare October 4, 2026 13:04
@Zaczero Zaczero closed this Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant