[Zaczero]

Motivation and Context

A scrub started while a replacement still has outstanding dirty time log
(DTL) ranges can detach the original device before repairing the replacement.

dsl_scan_setup_sync() already calls vdev_resilver_needed() to limit the
scan to those ranges, but previously retained POOL_SCAN_SCRUB when the
original request was a scrub. That mismatch can lose the usable copy of a
split indirect block: its segment reads have no block pointer with which to
verify a checksum, so the replacing vdev compares the original's valid bytes
with the incomplete replacement and reports ECKSUM. The indirect layer
discards the segment. Scan completion can subsequently clear the DTL and
detach the original while the replacement is still incomplete.

This was found while investigating zloop run 32644998517 for #18826.
The retained pool history shows a canceled rebuild, followed by
scan setup func=1 mintxg=3 maxtxg=940, scan errors, and detachment of the
original child.

Description

Record the effective scan as POOL_SCAN_RESILVER, allowing the existing
resilver path to select the valid source, repair the replacement, and account
for failed repair writes.

Persist DSF_SCRUB_REQUESTED to preserve the original request for scrub
status, pause/resume/cancel, wait, reopen, and thorough-scrub behavior.
The existing pool-history func field continues to describe the requested
scan type. A DTL-limited scan does not advance last_scrubbed_txg as though
it had completed a full scrub.

How Has This Been Tested?

Both regressions use a checkpoint to retain replacement DTLs after an initial
resilver. They disable resilver_defer to prevent an automatic retry from
consuming those DTLs before the tested scrub starts.

• replacement/scrub_resilver_classification checks scrub controls across a
  paused export/import, thorough-scrub resume, target selection, repair,
  replacement completion, events, and last_scrubbed_txg.
• replacement/scrub_resilver_split constructs split indirect mappings and
  an incomplete replacement. It verifies a mapped segment directly on each
  leaf before repair, then checks the file after replacement completion and
  export/import.

Both tests were exercised with debug kernel and userspace builds in an
isolated Debian 12 VM running Linux 6.1.0-53-cloud-amd64.

On the parent revision, the split-indirect regression established a good
original and an incomplete replacement. The scrub then reported an error,
cleared the replacement DTL, and detached the original. After export, import
failed with insufficient replicas; the debug log reported an unreadable
obsolete space map.

With the fix, the regression completes the replacement and verifies the
file contents and pool integrity with a post-import comparison and zdb.

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.
• All commit messages are properly formatted and contain Signed-off-by.

[Zaczero]

The remaining CI failures are unrelated.

[alek-p]

Thanks for bring this up this and submitting the PR. Your patch treats that leftover DTL as a scrub problem but it's really a symptom of resilver not completing in my oppinion. If I understand the problem correctly I think there is a tighter fix for this (not tested):

diff --git module/zfs/spa_checkpoint.c module/zfs/spa_checkpoint.c
index 870902237..84a4643ee 100644
--- module/zfs/spa_checkpoint.c
+++ module/zfs/spa_checkpoint.c
@@ -186,6 +186,13 @@ spa_checkpoint_discard_complete_sync(void *arg, dmu_tx_t *tx)

        spa_history_log_internal(spa, "spa discard checkpoint", tx,
            "finished discarding checkpointed state from the pool");
+
+       /*
+        * Resilver cannot excise DTL_MISSING while a checkpoint is active.
+        * Now that it is gone, finish any replacement that stalled.
+        */
+       if (vdev_resilver_needed(spa->spa_root_vdev, NULL, NULL))
+               spa_async_request(spa, SPA_ASYNC_RESILVER);
 }

 typedef struct spa_checkpoint_discard_sync_callback_arg {

The issue is that the checkpoint discard drops the reason for holding the DTL and then does nothing. The first resilver already walked the live tree and set DSS_FINISHED. Nothing starts a second pass, and problems follow from that.

[Zaczero]

@alek-p
I used a checkpoint in the test to keep the replacement DTLs outstanding.
I should have made that clearer in the PR description.

Starting a resilver after checkpoint discard could address that test setup.
The original failure, however, followed a canceled rebuild. The retained
history from zloop run 32644998517 shows:

txg 935: rebuild started
txg 940: replacement attached
txg 941: rebuild canceled
txg 986: scan setup func=1 mintxg=3 maxtxg=940
txg 989: scan done errors=26
txg 996: original replacement child detached

There is no checkpoint-discard operation in that retained sequence.

The bug is that scan setup limits the scrub to the DTL ranges needing repair,
but still uses scrub I/O. A split indirect segment has no block pointer for
checksum verification. The replacing vdev can therefore compare valid data
from the original with incomplete data from the replacement and return
ECKSUM. The indirect layer then discards the usable segment. Using resilver
I/O selects the valid source and repairs the replacement.

I've also added a regression with split indirect mappings and an incomplete
replacement. Before starting the scrub, it reads a mapped file segment
directly from both devices to check that the original has the correct data
and the replacement does not.

On the parent revision, this test reports a scan error, clears the replacement
DTL, and detaches the original. The pool then fails to import after export
because metadata is unreadable. With the fix, the replacement completes,
the pool imports, and the file comparison and zdb verification succeed.

[alek-p]

Sounds like the rebuild cancel path has the same issue as the checkpoint discard path? So adding something like this would help?

diff --git module/zfs/vdev_rebuild.c module/zfs/vdev_rebuild.c
index 7e857448d..4daf76fed 100644
--- module/zfs/vdev_rebuild.c
+++ module/zfs/vdev_rebuild.c
@@ -382,6 +382,9 @@ vdev_rebuild_cancel_sync(void *arg, dmu_tx_t *tx)
        vd->vdev_rebuilding = B_FALSE;
        mutex_exit(&vd->vdev_rebuild_lock);

+       /* Start a resilver if replacement DTLs remain. */
+       dsl_scan_assess_vdev(spa_get_dsl(spa), spa->spa_root_vdev);
+
        spa_notify_waiters(spa);
        cv_broadcast(&vd->vdev_rebuild_cv);
 }

It's hard to keep all the corner cases in my head but fundamentally I think a scrub is not responsible for DTL so should not be involved if we can avoid it?

[Zaczero]

Yes, the rebuild-cancel path needed the same healing reassessment as checkpoint discard. I followed that through, and your point about scrub ownership changed the direction of the fix: scrub completion should not authorize clearing missing-write DTLs.

I’ve dropped the approach of treating a DTL-limited scrub as a resilver while preserving its scrub identity. The replacement keeps verification and healing separate: a new scrub is rejected while available devices need healing, and missing-write ranges are retired only when the healing pass has sufficient coverage. It also includes reassessment after rebuild cancellation and checkpoint discard, as you suggested.

The investigation produced three PRs:

• dsl_scan: require healing coverage before retiring missing writes #19119, healing lifecycle: replaces this PR. It separates scrub admission from healing, preserves repair-error and coverage evidence across interruption and device return, and schedules healing after the relevant scan and topology transitions.
• vdev_indirect: validate split-block copies before repair #19120, split-indirect repair: addresses an additional problem underneath the original failure. Individual segments have no checksum, so subordinate mirror/replacing/spare code must not repair copies before the indirect layer validates the complete block.
• vdev_rebuild: account for failed repairs before retiring DTLs #19121, sequential rebuild: addresses failed destination writes missing from rebuild error accounting, retirement falling through to unrelated scan state, and a range-underflow problem caused by recalculating rebuild bounds after failures.