OpenZFS 7614 - zfs device evacuation/removal

[dweeezil]

Description

This project allows top-level vdevs to be removed from the storage pool
with "zpool remove", reducing the total amount of storage in the pool.
This operation copies all allocated regions of the device to be removed
onto other devices, recording the mapping from old to new location.
After the removal is complete, read and free operations to the removed
(now "indirect") vdev must be remapped and performed at the new location
on disk. The indirect mapping table is kept in memory whenever the pool
is loaded, so there is minimal performance overhead when doing
operations on the indirect vdev.

The size of the in-memory mapping table will be reduced when its entries
become "obsolete" because they are no longer used by any block pointers
in the pool. An entry becomes obsolete when all the blocks that use it
are freed. An entry can also become obsolete when all the snapshots
that reference it are deleted, and the block pointers that reference it
have been "remapped" in all filesystems/zvols (and clones). Whenever an
indirect block is written, all the block pointers in it will be
"remapped" to their new (concrete) locations if possible. This process
can be accelerated by using the "zfs remap" command to proactively
rewrite all indirect blocks that reference indirect (removed) vdevs.

Note that when a device is removed, we do not verify the checksum of the
data that is copied. This makes the process much faster, but if it were
used on redundant vdevs (i.e. mirror or raidz vdevs), it would be
possible to copy the wrong data, when we have the correct data on e.g.
the other side of the mirror. Therefore, mirror and raidz devices can
not be removed.

Motivation and Context

See above.

How Has This Been Tested?

Additions to the test suite in functional/removal.

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)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• All commit messages are properly formatted and contain Signed-off-by.
• Change has been approved by a ZFS on Linux member.

[dweeezil]

I'm posting this to give the bots a chance to confirm the few test suite failures I'm seeing as well as a general overall health-check.

The few test suite failures seem to be related to the inability of zdb to work properly when a removal operation is in progress and/ore related issues. I've not looked into this too deeply yet but it may very well be some sort of latent linux-ism.

There are also a number of areas I came across during the ports which need looking into.

That all said, this does seem to work in most of the tests and during my manual testing. Also, I'll note that it hasn't been merged upstream yet but I'm hoping this port will give some wider coverage and also allow me and, hopefully others, to review the upstream patch.

[EDIT] There may still be some large kmem allocations.

[EDIT 2] And at least one lingering style issue.

[dweeezil]

Ugh, that first round of tests was rather uninspiring! I'll get some of the basic stuff fixed up soon and re-push.

[dweeezil]

Since it's likely some of the issues uncovered here will have applicability in OpenZFS in general, anyone interested in following the progress of this should also follow the upstream PR in openzfs/openzfs#482.

[dweeezil]

Fixed a bunch of things, rebased to current master and synced with a couple of other new changes in the upstream PR. The removal_remap_deadlists is still failing for reasons I've not yet determined due to a strange failure with zdb.

[dweeezil]

Re-submitting shortly to give the ZoL buildbots another crack it this. My local tests are still failing on removal_remap_deadlists for reasons I've still not determined, but it appears to be some sort of timing issue when running zdb quickly following a pool configuration change. Manual testing of various removal scenarios all work properly as do all the other test suite tests.

[dweeezil]

Status update: There are 3 main issues the buildbots are running into:

• ztest hangs, seemingly due to Hangs in ztest due to /dev/random entropy exhaustion #7017
• ztest segfaulting at spa_remap_blkptr+0x139
• Linux's refusal to unmount file systems

Addressing these in order:

The two types of ztest problems seen by the buildbots seem to be hangs and segfaults in spa_remap_blkptr(). The only hangs I've been able to reproduce, particularly in KVM guest environments, are due to the issue outlined in #7017 so I suspect the same may be happening with the buildbots.

The segfault is, as best as I can tell, in this line:

vd->vdev_ops->vdev_op_remap(vd, offset, size, remap_blkptr_cb, &rbca);

which would likely indicate a bogus vdev; presumably one without .vdev_op_remap set. I've not been able to reproduce the problem but the buildbots seem to run into it pretty regularly. I'll ask about this in the upstream PR. In the mean time, I plan on doing some more code inspection to see whether there's an obvious path by which this can happen.

Finally, a bunch of the new tests for device removal assume that file systems can be cleanly unmounted while files are open and actively being written to. I added an -l (lazy unmount) option to a handful of commands (see e865f67) to try to work around the issues and it seems to help for my local tests but not always on the buildbots. I'd be interested in advice from anyone else with experience porting test suite scripts which may have also had this issue.

[dweeezil]

Another update: I've been able to locally reproduce both of the ztest issues seen by the buildbots.

[ahrens]

This just landed in illumos. Looking forward to seeing it in ZoL too! illumos/illumos-gate@5cabbc6

[behlendorf]

a bunch of the new tests for device removal assume that file systems can be cleanly unmounted while files are open and actively being written to

We've typically addressed this issue by slightly reworking to test cases to kill any active process in the mount point prior to issuing the unmount.

[megari]

We've typically addressed this issue by slightly reworking to test cases to kill any active process in the mount point prior to issuing the unmount.

fuser -km /mountpoint should do the trick, I think.

[dweeezil]

@behlendorf and @megari Thanks, I'll look into getting the processes killed that are preventing the unmount (actually zpool destroy, etc.).

Also, another update: I've fixed the segfault in ztest; it was caused by a new dnode-iterating loop which was not large dnode aware. The other ztest issue is a deadlock which, unfortunately, is a pain to diagnose with gdb. The deadlock seems to always involve a call to ztest_ddt_repair() when it tries to do rw_wrlock(&ztest_name_lock);.

Once the unmounting issue is handled, I think all the test suite should be good.

[behlendorf]

Sounds good!

The deadlock seems to always involve a call to ztest_ddt_repair() when it tries to do rw_wrlock(&ztest_name_lock);.

This is currently a preexisting known unrelated issue in master so it shouldn't hold up this PR. Let me go ahead and open a new issue for with the debugging I have and we can resolve it in a separate PR

[dweeezil]

I'm rather surprised at this type of failure on the "Amazon 2" buildbot:

Test: /usr/share/zfs/zfs-tests/tests/functional/removal/remove_raidz (run as root) [00:10] [FAIL]
22:48:20.41 SUCCESS: mkfile 268435456 /tmp/dsk1
22:48:20.75 SUCCESS: mkfile 268435456 /tmp/dsk2
22:48:21.11 SUCCESS: mkfile 268435456 /tmp/dsk3
22:48:21.11 NOTE: begin default_setup_noexit
22:48:30.01 SUCCESS: zpool create -f testpool /tmp/dsk1 raidz /tmp/dsk2 /tmp/dsk3
...
22:48:30.57 rm: cannot remove 'raidz': Is a directory
22:48:30.57 ERROR: rm -f /tmp/dsk1 raidz /tmp/dsk2 /tmp/dsk3 exited 1

I realize that it's probably a good idea to do "rm -f $DISKS", however, I've never seen any version of the "rm" program which fails on a nonexistent argument when run as "rm -f".

[EDIT] This seems to happen on other buildbot instance types.

[ivucica]

openzfs/openzfs#251 matches this PR's description in that removal of mirror devices is not supported.

This PR, however, refers to openzfs/openzfs#482 which omits mention of mirror devices in the PR description.

Is it intended that this PR #6900 supports removal of mirror devices?

Thanks :-)

[ivucica]

Also, @dweeezil: the failure seems to be:

22:48:30.57 rm: cannot remove 'raidz': Is a directory

It's not that an argument is missing, but that it's present as a directory.

[dweeezil]

I just pushed a refresh of this patch stack. The previous deadlocks were caused by the problem fixed in #7234. The issue of copying corrupted data from one leg of a mirror remains so the patch stack now contains a hack to ztest and zloop to not allow fault injection when device evacuation is enabled.

@ahrens, the problem fixed in openzfs/openzfs#561 has been ported in this patch stack. Could you please take a peek at a6de073, particularly vdev_mirror.c to see whether it looks like handled the ZoL differences correctly. Also, the openzfs/openzfs#561 patch does not fix the same problem I alluded to above. I made the extremely hack-ish script at https://gist.github.com/dweeezil/b17fd5c4c85100acaa1b12f77a11ca03 (which is likely filled with Linux-isms) to demonstrate that corruption on one half of a mirror can be copied to both halves of another mirror. It's not clear to me how this can work properly given that the DVAs created in spa_vdev_copy_segment() don't allow for checksums, however, it's also possible that I'm not fully understanding the whole evacuation process.

In any case, the last commit it this patch stack is intended to work around the issue in the context of ztest to see how everything checks out by the buildbots.

[dweeezil]

Given all the newly-appearing failures here (https://github.com/dweeezil/zfs/blob/illumos-7614/module/zfs/vdev_mirror.c#L628), I suspect the merge of openzfs/openzfs#561 may have been bad. At least I've been able to reproduce it locally and can investigate further.

Also, I suspect that most of the removal-related test suite failures are device busy issues when trying to export pools.

All in all, a rather uninspiring round of buildbot tests :|

[codecov]

Codecov Report

Merging #6900 into master will increase coverage by 0.34%.
The diff coverage is 88.43%.

@@            Coverage Diff             @@
##           master    #6900      +/-   ##
==========================================
+ Coverage   76.17%   76.51%   +0.34%     
==========================================
  Files         330      335       +5     
  Lines      104316   107018    +2702     
==========================================
+ Hits        79458    81886    +2428     
- Misses      24858    25132     +274

Flag	Coverage Δ
#kernel	76.95% <84.97%> (+1.1%)	⬆️
#user	66.07% <80.2%> (+0.59%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b0f5b2...0976df5. Read the comment docs.

[elpamyelhsa]

I'm a little lost, did this get accepted into the main release or is it waiting for something?

[Skaronator]

This has been merged in to the master branch and will be available on the next major release 0.8.0

[jdrch]

I'm looking into using ZFS and am a bit confused by the wording of the pull request. Specifically, this part:

mirror and raidz devices cannot be removed.

This seems to contradict the 1st sentence:

allows top-level vdevs to be removed from the storage pool with "zpool remove", reducing the total amount of storage in the pool.

Unless the differentiating factor between the 2 statements is the use of "device" vs. "vdev." Assuming this is the case, let's say I have a pool, Pool A:

Pool A:
vdev1 = array of diska and diskb
vdev2 = array of diskc and diskd
vdev3 = array of diske and diskf
vdev4 = diskg

Per how I read the above, I'd be able to remove any of the vdevs and have Pool A's size be reduced accordingly, but I would be unable to remove any of the individual disks from the pool except vdev4. Is that correct?

Following from the above, if I wanted to remove an individual disk from vdev1, vdev2, or vdev3, I'd have to remove the vdev containing it and then remove it from that vdev, but doing so would "destroy" that vdev as far as ZFS is concerned. Is that correct?

Just want to ensure I'm understanding this.

[behlendorf]

@jdrch the original text for this PR included some awkward wording and was incorrect in a few places. Let me refer you to the finalized wording in the man page which should help clarify the restrictions surrounding device removal.

     zpool remove [-np] pool device...
             Removes the specified device from the pool.  This command sup‐
             ports removing hot spare, cache, log, and both mirrored and non-
             redundant primary top-level vdevs, including dedup and special
             vdevs.  When the primary pool storage includes a top-level raidz
             vdev only hot spare, cache, and log devices can be removed.

             Removing a top-level vdev reduces the total amount of space in
             the storage pool.  The specified device will be evacuated by
             copying all allocated space from it to the other devices in the
             pool.  In this case, the zpool remove command initiates the
             removal and returns, while the evacuation continues in the back‐
             ground.  The removal progress can be monitored with zpool status.
             If an IO error is encountered during the removal process it will
             be cancelled. The device_removal feature flag must be enabled to
             remove a top-level vdev, see zpool-features(5).

             A mirrored top-level device (log or data) can be removed by spec‐
             ifying the top-level mirror for the same.  Non-log devices or
             data devices that are part of a mirrored configuration can be
             removed using the zpool detach command.

The key restriction worth emphasizing for device removal, is that no top-level data device may be removed if there exists a top-level raidz vdev in the pool. Only mirror and non-redundant vdevs can be added to the pool if you intend to use device removal.

If you have a specific scenario in mind, I'd encourage you to create a pool using sparse file vdevs and use them to run through the process.

[jdrch]

@behlendorf Super! Thanks!

[candlerb]

@behlendorf:

The key restriction worth emphasizing for device removal, is that no top-level data device may be removed if there exists a top-level raidz vdev in the pool. Only mirror and non-redundant vdevs can be added to the pool if you intend to use device removal.

Solaris ZFS got the ability to remove top-level vdevs in 2018:
https://blogs.oracle.com/solaris/oracle-solaris-zfs-device-removal
... and one of the examples explicitly shows a pool consisting of a raidz, then a top-level vdev being added to the pool, and removed again (see under the heading "Misconfigured Pool Device")

I was recently bitten by ZOL's limitation not to be able to do this. Someone else was trying to add a hot spare to a zpool, but they wrongly added it as a top-level device (using -f). Then it became impossible to remove; worse, it got used for real data, and was unprotected.