Fast Dedup: ZAP Shrinking #15888

allanjude · 2024-02-14T13:46:06Z

Motivation and Context

If ZAP records are deleted such that an entire leaf block of the ZAP object is emptied, the block is not reclaimed.
In addition to allowing directories and other users of ZAPs to shrink when a large number of objects are removed, this is required for Fast Dedup's DDT pruning feature.

Description

This change addresses this issue in part by noticing when adjacent blocks are emptied, and collapses them into a single empty block, thereby reducing the total amount of space in the ZAP object.

This adds a tuneable, zfs_zap_shrink_enabled, set by default. If disabled, the shrinking behaviour is disabled.

This replaces #14088

How Has This Been Tested?

TBD.

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)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil 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.

module/zfs/zap.c

amotin · 2024-03-02T19:36:44Z

module/zfs/zap.c

+		uint64_t sl_hash = ZAP_PREFIX_HASH(sl_prefix, prefix_len);
+		int slbit = prefix & 1;
+
+		ASSERT3U(zt_shift, ==, zap_f_phys(zap)->zap_ptrtbl.zt_shift);


We should re-read zt_shift from ZAP after reacquiring the ZAP lock below, where it may actually change, instead of asserting this here.

bhodgens · 2024-03-14T21:37:49Z

I'm extremely happy to see this one getting attention! This has been one of my 'pet bugs' for years.

amotin · 2024-03-19T12:48:44Z

module/zfs/zap.c

+		uint64_t sl_hash = ZAP_PREFIX_HASH(sl_prefix, prefix_len);
+		int slbit = prefix & 1;
+
+		ASSERT3S(zap_leaf_phys(l)->l_hdr.lh_nentries, ==, 0);


Suggested change

ASSERT3S(zap_leaf_phys(l)->l_hdr.lh_nentries, ==, 0);

ASSERT3U(zap_leaf_phys(l)->l_hdr.lh_nentries, ==, 0);

module/zfs/zap.c

amotin

Thanks you.

behlendorf

As part of this commit let's add explicit test coverage for at least one of the most common cases. Specifically creating a directory, adding a large number of files so it's converted to a fatzap, then removing those files and verifying the zap is collapsed.

The tests/zfs-tests/tests/functional/cp_files/cp_files_001_pos.ksh already does most of this and would be relatively easy to extend by using zdb check the zap stats.

Can you also add your signed off by when refreshing this.

module/zfs/zap.c

This allows ZAPs to shrink. When there are two empty sibling leafs, one of them is collapsed and its storage space is reused. This improved performance on directories that at one time contained a large number of files, but many or all of those files have since been deleted. This also applies to all other types of ZAPs as well. Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com>

Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com>

tests/runfiles/common.run

tests/zfs-tests/tests/functional/zap_shrink/zap_shrink_001_pos.ksh

Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com>

jumbi77 · 2024-04-25T20:25:11Z

Awesome that this got merged! Thanks to klarasystems & iX!

@behlendorf It is possible that this can be backported and released in a 2.2.x release, independent from the whole/complete fast dedupe feature?

behlendorf · 2024-04-25T20:54:39Z

@jumbi77 yes, but we'll want it to have had a lot of soak time in the master branch first.

jumbi77 · 2024-08-28T06:20:23Z

@jumbi77 yes, but we'll want it to have had a lot of soak time in the master branch first.

May i ask if around 4 month is enough soak time? @tonyhutter is it possible to pull this into current 2.2.6?

This allows ZAPs to shrink. When there are two empty sibling leafs, one of them is collapsed and its storage space is reused. This improved performance on directories that at one time contained a large number of files, but many or all of those files have since been deleted. This also applies to all other types of ZAPs as well. Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com> Closes openzfs#15888

behlendorf mentioned this pull request Feb 15, 2024

WIP - ZAP Shrinking #14088

Closed

behlendorf added the Status: Code Review Needed Ready for review and testing label Feb 15, 2024

robn force-pushed the fdt-rel-zap-shrink branch from a076382 to b9c2849 Compare February 15, 2024 19:58

amotin requested changes Mar 2, 2024

View reviewed changes

amotin reviewed Mar 19, 2024

View reviewed changes

amotin approved these changes Mar 19, 2024

View reviewed changes

robn mentioned this pull request Mar 27, 2024

[2.2] vdev_disk: rewrite BIO filling machinery to avoid split pages #16032

Merged

13 tasks

behlendorf approved these changes Apr 9, 2024

View reviewed changes

module/zfs/zap.c Show resolved Hide resolved

alex-stetsenko added 2 commits April 13, 2024 17:48

Add ZAP shrinking support (review fixes)

ee467c3

Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com>

alex-stetsenko force-pushed the fdt-rel-zap-shrink branch 3 times, most recently from fcd91d5 to 2599b25 Compare April 15, 2024 09:55

behlendorf reviewed Apr 15, 2024

View reviewed changes

alex-stetsenko force-pushed the fdt-rel-zap-shrink branch 3 times, most recently from a22f7d1 to 1c3c09a Compare April 19, 2024 18:07

behlendorf reviewed Apr 19, 2024

View reviewed changes

tests/zfs-tests/tests/functional/zap_shrink/zap_shrink_001_pos.ksh Outdated Show resolved Hide resolved

Add ZAP shrinking support (zfs-test, try openzfs#5)

297a274

Sponsored-by: iXsystems, Inc. Sponsored-by: Klara, Inc. Signed-off-by: Alexander Stetsenko <alex.stetsenko@klarasystems.com>

alex-stetsenko force-pushed the fdt-rel-zap-shrink branch from 1c3c09a to 297a274 Compare April 20, 2024 11:24

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 22, 2024

behlendorf merged commit 5044c4e into openzfs:master Apr 24, 2024
24 of 25 checks passed

amotin mentioned this pull request May 3, 2024

PANIC at zap.c:441:zap_create_leaf #16157

Closed

don-brady mentioned this pull request May 14, 2024

Fast Dedup: DDT Prefetch #15890

Merged

13 tasks

robn mentioned this pull request Jul 20, 2024

zfs-2.2.5 patchset #16359

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fast Dedup: ZAP Shrinking #15888

Fast Dedup: ZAP Shrinking #15888

allanjude commented Feb 14, 2024

amotin Mar 2, 2024

alex-stetsenko Mar 19, 2024

bhodgens commented Mar 14, 2024

amotin Mar 19, 2024

amotin left a comment

behlendorf left a comment

jumbi77 commented Apr 25, 2024 •

edited

Loading

behlendorf commented Apr 25, 2024

jumbi77 commented Aug 28, 2024

	ASSERT3S(zap_leaf_phys(l)->l_hdr.lh_nentries, ==, 0);
	ASSERT3U(zap_leaf_phys(l)->l_hdr.lh_nentries, ==, 0);

Fast Dedup: ZAP Shrinking #15888

Fast Dedup: ZAP Shrinking #15888

Conversation

allanjude commented Feb 14, 2024

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

amotin Mar 2, 2024

Choose a reason for hiding this comment

alex-stetsenko Mar 19, 2024

Choose a reason for hiding this comment

bhodgens commented Mar 14, 2024

amotin Mar 19, 2024

Choose a reason for hiding this comment

amotin left a comment

Choose a reason for hiding this comment

behlendorf left a comment

Choose a reason for hiding this comment

jumbi77 commented Apr 25, 2024 • edited Loading

behlendorf commented Apr 25, 2024

jumbi77 commented Aug 28, 2024

jumbi77 commented Apr 25, 2024 •

edited

Loading