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

Space checking of special device removal is too strict #9142

Closed
ahrens opened this issue Aug 8, 2019 · 2 comments · Fixed by #11329
Closed

Space checking of special device removal is too strict #9142

ahrens opened this issue Aug 8, 2019 · 2 comments · Fixed by #11329
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@ahrens
Copy link
Member

ahrens commented Aug 8, 2019

System information

Type Version/Name
Distribution Name Ubuntu
Distribution Version
Linux Kernel
Architecture
ZFS Version 0.8+
SPL Version

Describe the problem you're observing

When removing a special device, sometimes we get an error cannot remove <devname>: out of space, when there's actually sufficient free space. We're especially likely to hit this when the device to remove is mostly empty

The problem is that spa_vdev_remove_top_check() does not take into account the free space in the device to be removed. For example, imagine a pool with 1TB available space (in the normal class, per zfs list), and a special device of 1.1TB with 0.1TB allocated. Logically, there is plenty of free space to move the 0.1TB of allocated space. However, the check being performed is special_device_size (1.1TB) < normal_available_space (1TB) which fails.

The solution is probably to always add the free space of the special class to available (not just when there will be some devices left in the class after removal). In the above example, we should be checking special_device_size (1.1TB) < normal_available_space (1TB) + special_available_space (1TB) which will succeed. This is similar to the logic when removing a normal device, where we check removing_device_size < normal_available_space, where normal_available_space includes the free space on the removing device.

Describe how to reproduce the problem

The alloc_class_013_pos test operates very close to this limit, so making the dedup device slightly larger, or the zvol slightly larger, or the normal devices slightly smaller, will trigger this.

Include any warning/errors/backtraces from the system logs

static int
spa_vdev_remove_top_check(vdev_t *vd)
{
...
    /* available space in the pool's normal class */
    uint64_t available = dsl_dir_space_available(
        spa->spa_dsl_pool->dp_root_dir, NULL, 0, B_TRUE);

    metaslab_class_t *mc = vd->vdev_mg->mg_class;

    /*
     * When removing a vdev from an allocation class that has
     * remaining vdevs, include available space from the class.
     */
    if (mc != spa_normal_class(spa) && mc->mc_groups > 1) {
        uint64_t class_avail = metaslab_class_get_space(mc) -
            metaslab_class_get_alloc(mc);

        /* add class space, adjusted for overhead */
        available += (class_avail * 94) / 100;
    }

    /*
     * There has to be enough free space to remove the
     * device and leave double the "slop" space (i.e. we
     * must leave at least 3% of the pool free, in addition to
     * the normal slop space).
     */
    if (available < vd->vdev_stat.vs_dspace + spa_get_slop_space(spa)) {
        return (SET_ERROR(ENOSPC));
    }
@ahrens
Copy link
Member Author

ahrens commented Aug 8, 2019

cc @don-brady

@don-brady don-brady added the Type: Defect Incorrect behavior (e.g. crash, hang) label Aug 8, 2019
@don-brady don-brady self-assigned this Aug 8, 2019
@amotin
Copy link
Member

amotin commented Mar 9, 2020

I've found one more issue in the mentioned code: mc->mc_groups > 1 condition quoted above does not allow to remove special vdev in situation when there are two of them and capacity of normal vdev(s) alone is insufficient. For example, if md0, md1 and md2 have the same size, then this test fails, while it should not:

# zpool create qqq md0 special md1 special md2
# zpool remove qqq md2
cannot remove md2: out of space

ghost pushed a commit to zfsonfreebsd/ZoF that referenced this issue Mar 10, 2020
Issue openzfs#9142 describes an error in the checks for device removal that
can prevent removal of special allocation class vdevs in some
situations.

Enhance alloc_class/alloc_class_012_pos to check situations where this
bug occurs.

Update zts-report with knowledge of issue openzfs#9142.

Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
ghost pushed a commit to zfsonfreebsd/ZoF that referenced this issue Mar 10, 2020
Update zts-report with knowledge of issue openzfs#9142
behlendorf pushed a commit that referenced this issue Mar 12, 2020
Issue #9142 describes an error in the checks for device removal that
can prevent removal of special allocation class vdevs in some
situations.

Enhance alloc_class/alloc_class_012_pos to check situations where this
bug occurs.

Update zts-report with knowledge of issue #9142.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10116 
Issue #9142
ahrens added a commit to ahrens/zfs that referenced this issue Dec 11, 2020
The space in special devices is not included in spa_dspace (or
dsl_pool_adjustedsize(), or the zfs `available` property).  Therefore
there is always at least as much free space in the normal class, as
there is allocated in the special class(es).  And therefore, there is
always enough free space to remove a special device.

However, the checks for free space when removing special devices did not
take this into account.  This commit corrects that.

Additionally, when removing the 2nd-to-last special device, its space
would not be reallocated to the last remaining special device, because
mc_groups has already been decremented.  That is also fixed.

Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
Closes openzfs#9142
jsai20 pushed a commit to jsai20/zfs that referenced this issue Mar 30, 2021
Issue openzfs#9142 describes an error in the checks for device removal that
can prevent removal of special allocation class vdevs in some
situations.

Enhance alloc_class/alloc_class_012_pos to check situations where this
bug occurs.

Update zts-report with knowledge of issue openzfs#9142.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10116 
Issue openzfs#9142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants