limit the dedup min size 64K for less fragment?

[wangyugui-e16]

Hi,

Is it OK to limit the dedup min size 64K for less fragment?

for SSD, it is OK to dedup small fragement such as 4K, but for HDD, there is no need to dedup small fragement such as 4K to have 1 more fragment?

[wangyugui-e16]

Is this patch OK?

0002-dedup-only-64K-or-bigger.patch

[Zygo]

No, because btrfs will not remove an extent that has any remaining block references, even if blocks in the extent are no longer reachable. bees computes all of the data copy and dedupe operations needed to eliminate the entire extent from the filesystem, including creating temporary extents with copies of unique data so that all blocks can be deduped. This patch will prevent some of the dedupe commands from being executed, which will result in both the original and temporary copy extents becoming permanently stored in the filesystem. In other words, the patch won't have a significant effect on fragmentation (other dedupe operations will still execute, causing fragmentation by omitting dedupe of some small extents in between large ones), but will significantly increase space usage.

Filtering dedupes by size requires a two-pass algorithm: one pass to compute a complete solution for all the copy and dedupe sizes for an extent, and another to perform either all of the copy and dedupe actions for the extent or none of them, depending on predicted fragmentation outcome. BeesContext::scan_one_extent isn't structured that way (it does all dedupes first, then fills in the gaps with copies), and it's easier to rewrite the entire function (and its support functions in bees-resolve.cc and bees-types.cc) than to fix it. This rewrite is already in progress, because there is a long list of problems with scan_one_extent and excessive fragmentation is only one of them.

In the short term, the safest version of a minimum dedupe size is the commented-out return in scan_one_extent, which won't increase disk space usage, and will still dedupe a small extent if it is the only extent within a file (so it doesn't cause fragmentation):

BeesFileRange
BeesContext::scan_one_extent(const BeesFileRange &bfr, const Extent &e)
{

...

        // EXPERIMENT:  Don't bother with tiny extents unless they are the entire file.
        // We'll take a tiny extent at BOF or EOF but not in between.
        if (e.begin() && e.size() < 128 * 1024 && e.end() != Stat(bfr.fd()).st_size) {
                BEESCOUNT(scan_extent_tiny);
                // This doesn't work properly with the current architecture,
                // so we don't do an early return here.
                // return bfr;
        }

If your stats counters are showing large numbers of scan_extent_tiny events, you might want to try uncommenting the return statement.

If you are averse to fragmenting larger extents, you could also add:

                        BeesTracer::set_silent();
                        throw runtime_error("no copies");

to the beginning of BeesContext::rewrite_file_range, though be advised this will come with a significant loss of dedupe efficiency, as no dedupes within a partially matched extent will be possible. The worst case for this change is zero dedupe, as opposed to increasing disk space usage. Even this change isn't perfect--some fragmenting dedupes will still be done, it will only stop at the first copy, and it may cause problems with unreachable extents if some of the duplicated files are deleted.