Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't excise DTLs from unreadable vdev #2613

Closed
wants to merge 1 commit into from
Closed

Conversation

nedbass
Copy link
Contributor

@nedbass nedbass commented Aug 18, 2014

An offline or unreadable vdev may trip the following assertion
during a resilver scan in ztest:

ztest: ../../module/zfs/vdev.c:1746: Assertion
`scn->scn_phys.scn_min_txg <= vdev_dtl_min(vd) (0x4e3 <= 0x3)' failed.
child died with signal 6

The following analysis is by George Wilson:

The current scan is only resilvering a few txgs [70f, 711] but yet
this vdev has a min txg of 3. The problem is that this vdev is
currently not readable and as a result when the scan that was doing
the resilver it actually finished but didn't copy any of the data to
this device.

Now a second scan comes through and the device is still offline (ie.
not readable) so once again this device was did not have any data
copied over to it. This time when we check if we should excise the
DTLs from this device we determine we should since the scan is for a
txg much higher than the max value in this device's dtl range but we
end up tripping over this assertion:

    /*
     * When a resilver is initiated the scan will assign the
     * scn_max_txg
     * value to the highest txg value that exists in all DTLs. If
     * this
     * device's max DTL is not part of this scan (i.e. it is not in
     * the range (scn_min_txg, scn_max_txg] then it is not eligible
     * for excision.
     */
    if (vdev_dtl_max(vd) <= scn->scn_phys.scn_max_txg) {
            ASSERT3U(scn->scn_phys.scn_min_txg, <=, vdev_dtl_min(vd));

If the device is not readable than we don't want to ever excise any
of its dtls so we should return B_FALSE and not even bother with
anything further.

References: https://www.illumos.org/issues/4890

Issue #2302

Signed-off-by: Ned Bass [email protected]

An offline or unreadable vdev may trip the following assertion
during a resilver scan in ztest:

ztest: ../../module/zfs/vdev.c:1746: Assertion
`scn->scn_phys.scn_min_txg <= vdev_dtl_min(vd) (0x4e3 <= 0x3)' failed.
child died with signal 6

The following analysis is by George Wilson:

 The current scan is only resilvering a few txgs [70f, 711] but yet
 this vdev has a min txg of  3. The problem is that this vdev is
 currently not readable and as a result when the scan that was doing
 the resilver it actually finished but didn't copy any of the data to
 this device.

 Now a second scan comes through and the device is still offline (ie.
 not readable) so once again this device was did not have any data
 copied over to it. This time when we check if we should excise the
 DTLs from this device we determine we should since the scan is for a
 txg much higher than the max value in this device's dtl range but we
 end up tripping over this assertion:

        /*
         * When a resilver is initiated the scan will assign the
         * scn_max_txg
         * value to the highest txg value that exists in all DTLs. If
         * this
         * device's max DTL is not part of this scan (i.e. it is not in
         * the range (scn_min_txg, scn_max_txg] then it is not eligible
         * for excision.
         */
        if (vdev_dtl_max(vd) <= scn->scn_phys.scn_max_txg) {
                ASSERT3U(scn->scn_phys.scn_min_txg, <=, vdev_dtl_min(vd));

 If the device is not readable than we don't want to ever excise any
 of its dtls so we should return B_FALSE and not even bother with
 anything further.

References: https://www.illumos.org/issues/4890

Issue openzfs#2302

Signed-off-by: Ned Bass <[email protected]>
@nedbass
Copy link
Contributor Author

nedbass commented Aug 18, 2014

@grwilson is this patch what you had in mind in your analysis? Thanks

@behlendorf behlendorf added this to the 0.6.4 milestone Aug 19, 2014
@behlendorf behlendorf added the Bug label Aug 19, 2014
@nedbass
Copy link
Contributor Author

nedbass commented Aug 19, 2014

According to my testing this patch is not sufficient to avoid tripping the assertion. I instrumented the code to print the vdev state when this would hit, and it was online and readable. That is, vd->vdev_offline was 0 and vdev_readable(vd) returned true.

@behlendorf
Copy link
Contributor

@nedbass Wouldn't there be similar issues if the vdev was readable but not writable? It would be useful to dump more information about the vdev when the ASSERT is hit.

@nedbass
Copy link
Contributor Author

nedbass commented Aug 20, 2014

I did look into that and added more instrumentation. These fields were all zero

        uint64_t        vdev_offline;   /* persistent offline state     */         
        uint64_t        vdev_faulted;   /* persistent faulted state     */         
        uint64_t        vdev_degraded;  /* persistent degraded state    */         
        uint64_t        vdev_removed;   /* persistent removed state     */ 

and vd->vdev_state was VDEV_STATE_HEALTHY. I didn't check vdev->vdev_cant_write though, but I suspect it will be 0 as well.

@behlendorf
Copy link
Contributor

OK, clearly we're going to need to investigate this some more. But since the patch as proposed doesn't address this issue I'm closing this out.

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.

2 participants