[feature request] implement delphix device removal

[mailinglists35]

I've created this to track progress on
http://blog.delphix.com/alex/2015/01/15/openzfs-device-removal/
If you know more, please add links or comments.

[mailinglists35]

@ahrens said that outside help is needed: https://twitter.com/mahrens1/status/665006514411806720

[thegreatgazoo]

Link to the code: delphix/delphix-os@db775ef

[mailinglists35]

@behlendorf is there any chance this feature can be lobbied in the developers' circles? this feature would make zfs even more awesome! it's a bit frustrating to have to destroy and recreate pools in order to remove a device or vdev from a pool. even an offline version would be sufficient, there is no need for live removal.

[thegreatgazoo]

According to @ahrens, in addition to the original commit mentioned above:

• Fixes after commit:
  • commit 2882ebdc579bc734acca360645ed353dc96ce847
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Fri Dec 12 08:29:16 2014 -0800
    DLPX-34096 disable zfs_remap_blkptr_enable until it works right
  • commit f3de7c1c3e6b38c31e16aeb709e5d7e5d310acd6
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Wed Dec 17 00:21:27 2014 -0500
    DLPX-34061 metaslab_free_concrete() always verifies frees
  • commit 785e334e53cb7c52b211dd84406d4b6a097f61c4
    Author: Alex Reece alex@delphix.com
    Date: Tue Jan 6 09:10:00 2015 -0800
    DLPX-34092 Memory leak from zio_cache
  • commit cfdf7910e64199b8d40cf09dc1f129b9a892b45f
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Thu Jan 15 22:55:04 2015 -0800
    DLPX-34095 panic in lz4_decompress() due to device removal
  • commit dc60113301c9a136a409c7f74473844ae3a26450
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Thu Dec 18 09:03:47 2014 -0500
    DLPX-34098 ASSERT failed in spa_remove_init(): svr->svr_vdev->vdev_removing
• A follow-on feature which enables us to shrink the size of the device mapping:
  • commit 3487dba86ab348f578d428d1d8a1f887ed314009
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Tue Jun 16 10:48:47 2015 -0700
    DLPX-36664 track refcount of indirect mappings
  • commit 859bf791cb10e3c703f0895e3ff31fe2a5bad802
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Thu Jul 16 09:20:49 2015 -0700
    DLPX-38656 Device removal in 4.2 and upgrade to 4.3 results in reboot loop
  • commit b86c4d2eedcbadb37808f5c53235710fee6d44c9
    Author: Matthew Ahrens mahrens@delphix.com
    Date: Fri Jul 24 09:51:25 2015 -0700
    DLPX-38373 removal/removal_remap_deadlists sporadically fails
  • commit cb22f926f7a3f1914c7c5ceb0c0ee581403998ab
    Author: Joe Stein joe.stein@delphix.com
    Date: Mon Jul 13 20:16:49 2015 -0700
    DLPX-38166 Move obsolete refcounts of indirect mappings from vdev
    label to ZAP
    DLPX-38319 ztest: mismatch in obsolete refcounts

[awnz]

Excuse me barging in (it's just that I could have done with this feature after accidentally adding a stripe instead of a mirror during a multi-TB data migration exercise! Set back several days now)...

Do I have it right that this is now implemented upstream?
https://rudd-o.com/linux-and-free-software/delphix-zfs-based-on-open-zfs-now-supports-device-removal

[ahrens]

The post you referenced is correct in that it's in DelphixOS, but that's not really "upstream" for ZoL. Upstream is OpenZFS/illumos, where this feature is not yet integrated, but there is a pull request out for it: openzfs/openzfs#251 (Note that the PR supercedes the list of Delphix commits earlier in this thread.)

We would appreciate any help with porting it to ZoL!

[awnz]

hrm, I should really go and learn C so I can be more useful with these things. I might set that as a goal for this year so I may be more use in the future.

Anyways in my case, the send/destroy/recreate/receive is going faster than I thought it would.

As an alternative solution to the problem I hit (which I understand to be one of the biggest ZFS gotchas), as a layer of protection against accidentally and irreversibly adding single-disc/non-redundant stripes to a pool, what about the possibility of having the zpool command refuse to stripe by default? Say, if there were a module option safe_add, if it were set to 1, zpool add and zpool create would insist on use of a new vdev group keyword "stripe" if no other vdev group keyword were given before specifying individual vdevs to add.

(I hope I have my terminology right there, correct me if wrong).

ie, instead of zpool add tank sdx, I'd have to specify zpool add tank stripe sdx if I really, really, wanted to add a nonredundant disc to my array.

Has something like this been discussed before, or do you think it's worth discussing the possibility of such a change?

(edited to fix typos and clarify)

[ahrens]

@awnz ZFS already has this type of protection built in. zpool add will by default not let you add non-redundant (striped) vdevs to a redundant (e.g. mirror / RAIDZ) pool.

If you make a mistake with zpool create, hopefully you can realize it before putting too much data on it, and then you can start over with zpool create mirror ....

# zpool list -v test
NAME         SIZE  ALLOC   FREE  EXPANDSZ   FRAG    CAP  DEDUP  HEALTH  ALTROOT
test        7.94G   108K  7.94G         -     0%     0%  1.00x  ONLINE  -
  mirror    7.94G   108K  7.94G         -     0%     0%
    c1t1d0      -      -      -         -      -      -
    c1t2d0      -      -      -         -      -      -
# zpool add test c1t3d0
invalid vdev specification
use '-f' to override the following errors:
mismatched replication level: pool uses mirror and new vdev is disk

[awnz]

Oh crap, did I do -f? (history | grep "zfs add") CRAP, I did.

Oh now I feel stupid. I must have misread the error message. I'm going to quietly sneak out of the room now. Lesson learned: read and understand those error messages, don't be so quick to use -f

This was on home data. Lesson learned for when I implement this at work...

[behlendorf]

Closing. The device removal feature has been merged to OpenZFS and is being ported to ZoL in PR #6900.