add sysctls for arc shrinking and growing values
ClosedPublic

Authored by nikita_elyzion.net on Mon, Aug 28, 1:03 PM.

Details

Summary

The default value for arc_no_grow_shift may be not the best when using several hundred of GiB ARC and should be exposed as a sysctl.

Alternatively, it should be possible to modify arc_grow_retry to achieve the same thing.
Note that the current default value of 60sec for arc_grow_retry seems to be too big for intensive use of ZFS, at least in my use case.

Also, we should consider to make zfs_arc_overflow_shift modifiable too but I'm unsure if we need to force it to be smaller than arc_shrink_shift or not (which is already exposed as a sysctl).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bapt added a reviewer: will.
bapt added a reviewer: manu.Mon, Aug 28, 1:50 PM

However you generated this DR, Phabricator didn't get enough context info. The best way to start a code review is to use php5-arcanist according to the instructions at https://wiki.freebsd.org/Phabricator . But if you must use the web interface, remember to use "-D99999" when creating the diff to ensure that maximal context is available.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
407 ↗(On Diff #32442)

Wait time in what units?

However you generated this DR, Phabricator didn't get enough context info. The best way to start a code review is to use php5-arcanist according to the instructions at https://wiki.freebsd.org/Phabricator . But if you must use the web interface, remember to use "-D99999" when creating the diff to ensure that maximal context is available.

As you guessed, I'm not using arc (yet), I have updated the diff with enough context.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
407 ↗(On Diff #32442)

in seconds, updated.

bapt accepted this revision.Tue, Aug 29, 8:35 AM

Looks good to me, but I will wait for people with more internal knowledge than I have about ZFS to step in anyway

This revision is now accepted and ready to land.Tue, Aug 29, 8:35 AM
manu accepted this revision.Tue, Aug 29, 9:04 AM
In D12144#252281, @bapt wrote:

Looks good to me, but I will wait for people with more internal knowledge than I have about ZFS to step in anyway

Same for me.

Nikita: Could you provide numbers ? (i.e.: when does one need to change the default value)

In D12144#252284, @manu wrote:
In D12144#252281, @bapt wrote:

Looks good to me, but I will wait for people with more internal knowledge than I have about ZFS to step in anyway

Same for me.

Nikita: Could you provide numbers ? (i.e.: when does one need to change the default value)

Yes of course, let's take this list of settings which impacts the arc_reclaim and arc_no_grow

vfs.zfs.arc_shrink_shift (default 7, ~2Gib for 256G)
vfs.zfs.arc_grow_retry (new) (default 60sec)
vfs.zfs.arc_no_grow_shift (new) (default 5, ~8GiB for 256G)
vfs.zfs.arc_overflow_shift (should be added too?) (default 8, ~1Gib for 256G)

When you detect that you have : arc_available_memory() < 0 (in my case, on the vm_lowmem event), you start to free some memory and you set arc_no_grow to B_TRUE (the amount of memory to reclaim seems to be at least arc_c >> vfs.zfs.arc_shrink_shift).
As long as arc_no_grow is true, your arc wont grow (obvious).
Then (we are in a loop), as long as you have not freed at least arc_c >> vfs.zfs.arc_no_grow_shift ram, you continue to set the arc_no_grow to B_TRUE.
Then, when those both conditions are satisfied, you wait arc_grow_retry time before setting back arc_no_grow to false (60 sec, which could be pretty long depending on how much your arc was shrinked).

Initially I just wanted to make arc_grow_retry a sysctl, then I also added arc_no_grow_shift since it seems to be useful to modify it too if you modify arc_shrink_shift.

smh requested changes to this revision.Thu, Aug 31, 11:36 AM
smh added a subscriber: smh.
smh added inline comments.
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c
398 ↗(On Diff #32445)

Style nit, needs wrapping.

This revision now requires changes to proceed.Thu, Aug 31, 11:36 AM
mav accepted this revision.Thu, Aug 31, 11:38 AM

I have no objections. I suppose that code came from Illumos, and there it is usual to modify kernel variables with debugger without sysctls. Adding sysctl would be a good FreeBSD alternative. If somebody wish to shoot his foot -- he should be able to do it.

This revision was automatically updated to reflect the committed changes.