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

auto compression #5928

Closed
wants to merge 1 commit into from
Closed

auto compression #5928

wants to merge 1 commit into from

Conversation

n1kl
Copy link

@n1kl n1kl commented Mar 27, 2017

As part of my master thesis at University of Hamburg I have targeted to improve ZFS through compression. Now I would like to share my 3 feature branches with the community.

  1. lz4fast lz4fast compression #5927
  2. autocompression (current)
  3. qos Quality of service for ZFS + improvement through compression #5929

Description

This patch adds auto as ZFS compression type.
zfs set compression=auto

Motivation and Context

Which compression algorithm is best for high throughput? The answer to this depends on the type of hardware in use.
If compression takes long then the disk remains idle. If compression is faster than the writing speed of the disk then the CPU remains idle as compression and writing to the disk happens in parallel.
Auto compression tries to keep both as busy as possible.
The disk load is observed through the vdev queue. If the queue is empty a fast compression algorithm like lz4 with low compression rates is used and if the queue is full then gzip-[1-9] can require more CPU time for higher compression rates.
The already existing zio_dva_throttle might conflict with the concept described above. Therefore it is recommended to deactivate zio_dva_throttle.

Benchmark

Copy file from Tempfs to ZFS

8 Cores:

Name Ratio MB/s
auto 0.44 245
gzip-1 0.43 255
lz4 0.58 195
off 1 99

1 Core:

Name Ratio MB/s
auto 0.56 151
gzip-1 0.43 51
lz4 0.58 179
off 1 99

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)

Branch overlapping changes (feature, compress values)

The patch is has read-only backward compatibility by using the new introduced SPA_FEATURE_COMPRESS_AUTO feature. The feature activation procedure is equivalent to my other code branches.
Regarding the limited namespace of BP_GET_COMPRESS() (128 values), the
zio_compress enum's first part is for block pointer & dataset values, the second part for dataset values only. This is an alternative suggestion to #3908.

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.
  • Change has been approved by a ZFS on Linux member.

@behlendorf behlendorf added the Type: Performance Performance improvement or performance problem label Mar 27, 2017
@ahrens
Copy link
Member

ahrens commented Mar 27, 2017

This is a neat idea.

I don't see how it conflicts with zio_dva_throttle. The allocation throttle should be configured to send at least enough i/os to each device to fill its queue (zfs_vdev_async_write_max_active i/os), so I wouldn't think it would impact your measurement. But if it's true that your code is incompatible with the zio_dva_throttle, you'll need to resolve that before integration.

I would suggest using separate enums for the blkptr_t's compression field, and the property value's compression field, as is done in #3908

pool.

This feature becomes \fBactive\fR as soon as it is used on one dataset and will
return to being \fBenabled\fB once all filesystems that have ever had their compression set to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo, should be enabled\fR

@n1kl n1kl force-pushed the autocompression branch 3 times, most recently from df94bbc to 7cd574f Compare March 29, 2017 15:35
@n1kl
Copy link
Author

n1kl commented Mar 29, 2017

@ahrens
Thank you for reviewing.

For performance it is important that the vdev queue never gets empty.
To ensure this the auto algorithm needs a minimum queue size (in Bytes) to start as seen in the code below (module/zfs/compress_auto.c [61]).

uint32_t max_queue_depth = zfs_vdev_async_write_max_active * zfs_vdev_queue_depth_pct / 100;

/* keep at least 25 ZIOs in queue * compression factor about 2 = average 50 */

uint64_t buffer = size * (max_queue_depth / 4);
if (vd_queued_size_write >= buffer) {
vd_queued_size_write -= buffer;
} else {
vd_queued_size_write = 0;
}

If the compression factor is >4 there would have to be more than 100 ZIOs in the queue, the zio_dva_trottle becomes active and we would stick to lz4.
I wanted to point out this possible interference.
Reducing the queue buffer increases compression but increases the risk for the queue to become empty if there is high variance in the compression duration.

An alternative approach is to monitor vqc_active to count the number of ZIOs inside the queue but this is more risky because of variable ZIO size for compression and disk block size.

@jumbi77
Copy link
Contributor

jumbi77 commented Jun 24, 2017

Is that a similar feature like the smart compression (https://reviews.csiden.org/r/266/) ? btw does anyone know why this was discsrded?

@n1kl
Copy link
Author

n1kl commented Jun 26, 2017

@jumbi77 From reading the description of smart compression the compression is turned off for a while when data is classified incompressible to save computational resources. This is a further improvement that can also be added for this feature in the future.
At the moment the main focus is on automatically choosing a faster compression algorithm for faster storage devices, lower CPU power and high compressible data and a slower algorithm for slower storage devices, lower CPU power and low compressible data to achieve the best possible throughput.

@RubenKelevra
Copy link

Thanks for taking the time and effort to implement this, I bet this will majorly improve the usability of compression on desktop computers.

Did you test if this improves the system load while saving files of the system is busy? This is one of the major blockers IMHO for compression.

Does it support more than gzip-1 at the moment? Usually gzip-5 or even gzip-7 often gives a lot better compression on stuff like text files or software libraries.

This questions are just out of curiosity and shall not block any attempts of merging this, when rebase is completed.

@RubenKelevra
Copy link

RubenKelevra commented Apr 8, 2018

So, I digged in your code to answer the question. We have support for gzip compression offloading, so there might be machines out there which can handle much faster compression with gzip than before and gzip1 might not fully utilize the cards. Is it possible and reasonable to add more than one gzip compression level?

How do you think about renaming this feature, I feel auto compression might need some more explanation than for example adaptive compression.

What do you (all) think about changing the default for new pools to enable this feature? It should increase the performance without hurting system performance, CPU wise, much.

What is your opinion on this, @behlendorf?

@behlendorf behlendorf added the Status: Inactive Not being actively updated label Sep 25, 2018
@ahrens ahrens added the Status: Revision Needed Changes are required for the PR to be accepted label Sep 27, 2018
@ahrens
Copy link
Member

ahrens commented Jan 8, 2019

superseded by #7560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Inactive Not being actively updated Status: Revision Needed Changes are required for the PR to be accepted Type: Performance Performance improvement or performance problem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants