Check that THP is not set to always (madvise is ok)

[badboy]

THP can also be set to madvise, in which case it shouldn't cause
problems for Redis.

Fixes #3895

[reneklacan]

@badboy @antirez should this be merged?

[antirez]

Thanks @badboy, please could you make != NULL explicit? Otherwise certain compiler versions may complain with a warning.

[badboy]

Force-pushed with the changes.

[volkertb]

@badboy Wait a minute. According to this blog at DigitalOcean, using Redis with madvise actually caused memory release problems: https://blog.digitalocean.com/transparent-huge-pages-and-alternative-memory-allocators/

Wouldn't that mean that both "enabled" and "madvise" are problematic options and that the current warning as shown by Redis (with the recommendation to completely disable THP by setting it to "never") is actually justified? Or did I misread the blog?

[badboy]

From my limited understanding of madvise it should work. I have not (yet) read the DO blog post though.

[badboy]

@volkertb I read the article and the problem there is caused by huge-pages, not by the madvise setting for THP.
/sys/kernel/mm/transparent_hugepage/enabled allows for three settings:

• always:: Always use THP
• never: Never use THP
• madvise: Let user programs opt-in to THP using the madvise functions with the MADV_HUGEPAGE flag. See madvise(2).

As neither Redis nor the bundled jemalloc will use the MADV_HUGEPAGE flag, Redis won't use THP.

[volkertb]

OK, thanks for checking and clarifying this, @badboy. I hope this pull request gets reviewed and approved soon. 🙂

[minus7]

With the kernel option set to madvise, khugepaged will still be running and possibly cause latency issues. Thus checking for [never] might actually make sense.

Another thing: why are the server and the latency report using different ways to detect THP?

[antirez]

Based on this thread I would rather be strict than wrong and leave things as they are, requiring [never]...

[wbolster]

i think @badboy is correct and that this pr can be merged as-is.

when the setting is set to madvise, applications must opt-in. redis does not opt-in. that means that the warning only makes sense when the setting is set to always. and this pr changes exactly that.

[aguilarm]

Ran across this when adding redis to a kubernetes cluster. Since it's a host/node level modification I would prefer using [madvise] as the host may be home to many more processes than redis.

[volkertb]

So that means this PR can now be merged, right? Or does it still require additional code reviews?

[akumria]

@antirez Is the decision to leave this open then? Personally I've had customers ask about the message (when the value is set to 'madvise'). It is annoying THP doesn't impact redis (or jemalloc) and the theory that hugepaged causes latency issues when 'madvise' is set, is just that.

Theory.

There is no information, that I am aware of, that having the value set to 'madvise' and there being no THP consumers can cause a latency issue in redis.

[minus7]

Assuming whoever changes the kernel config to madvise knows what they're doing, only showing the message when set to always seems fine. Maybe adjust the message with a hint about madvise.

[czka]

I wonder how Google solved this for their "Memorystore" https://cloud.google.com/memorystore/docs/redis/. Maybe Redis team could leverage their Google Cloud partnership https://redislabs.com/partner/google/ and share the information?

FWIW, the 190 Google Container Engine VMs I checked (COS images) all have /sys/kernel/mm/transparent_hugepage/enabled set to madvise. I don't want to change this blindly to never and risk side effects for other workloads only to satisfy a (probably incorrect?) Redis hint about never being the only setting allowed.

[gjcarneiro]

Could redis maybe use this option?

       MADV_NOHUGEPAGE (since Linux 2.6.38)
              Ensures that memory in the address range specified by addr and
              length will not be collapsed into huge pages.

This way, even if THP is always,it should in theory disable hugepages for redis memory allocations?...

[hyperknot]

So what is the conclusion of this? Is the DO blog post valid for normal use cases? Like a stock Ubuntu 18.04 install?

Since Ubuntu 18.04 madvise is the default both for transparent_hugepage/enabled and /defrag, and configuring it is really not simple anymore (no rc.local supplied, systemd unit needs to be created), it'd be a huge help for installlations if the default madvise could be used.

[oranagra]

@badboy considering the feedback we got from Jason in #3895 (comment)
do you wanna complement this PR ans also change the text that suggests:
echo never > /sys/kernel/mm/transparent_hugepage/enabled