Add maxmemory-reserved-scale parameter for evicting key earlier to avoid OOM

[hwware]

Reference: #742 and https://learn.microsoft.com/en-us/azure/azure-cache-for-redis/cache-best-practices-memory-management (Azure)

Generally, when clients set maxmemory-policy as allkeys-lru or other memory eviction policies, and maxmemory as well, If server runs as write-heavy workloads, the data stored in memory could reach the maxmemory limit very quickly, then OOM message will be reported.

If we have maxmemory-reserved-scale parameter and client enable the lazyfree-lazy feature, for example, we can set maxmemory-reserved-scale as 0.8, it means key eviction begin when used memory reaches 8GB if maxmemory is 10GB, thus, server could continue process clients data and OOM will not happen at this time.

Thus, we can see the benefit is we can delay OOM time, but we need pay for less memory available.

One example for this paramter：
Assume

maxmemory 4GB
maxmemory-reserved-scale 20

Then we could check the detail by info memory command:

maxmemory:4294967296
maxmemory_human:4.00G
maxmemory_policy:allkeys-lru
maxmemory_reserved_scale:20
maxmemory_available:3435973836
maxmemory_available_human:3.20G

We could also update and get the maxmemory-reserved-scale value during runtime as following:

config set maxmemory-reserved-scale value
config get maxmemory-reserved-scale

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 20 lines in your changes missing coverage. Please review.

Project coverage is 70.51%. Comparing base (4986310) to head (9c3068b).
Report is 3 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/config.c	10.00%	9 Missing ⚠️
src/evict.c	60.00%	8 Missing ⚠️
src/module.c	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #831      +/-   ##
============================================
- Coverage     70.68%   70.51%   -0.18%     
============================================
  Files           115      115              
  Lines         63177    63209      +32     
============================================
- Hits          44657    44569      -88     
- Misses        18520    18640     +120     

Files with missing lines	Coverage Δ
src/server.c	87.65% <100.00%> (-0.02%)	⬇️
src/server.h	100.00% <100.00%> (ø)
src/module.c	9.64% <0.00%> (ø)
src/evict.c	94.56% <60.00%> (-3.19%)	⬇️
src/config.c	78.39% <10.00%> (-0.45%)	⬇️

... and 15 files with indirect coverage changes

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[enjoy-binbin]

this seem like a interesting feature, did not review, just drop a comment that approve the concept.

[madolson]

I also like the idea. I just haven't really spent enough time thinking about it. Memory management is a big area in Valkey we need to improve.

[PingXie]

Took a quick look.

[PingXie]

+1 on introducing "back pressure" earlier. I feel that this could be used with `maxmemory_eviction_tenacity" to give an even smoother eviction experience.

[PingXie]

Thanks @hwware! LGTM overall. Can you add some tests too?

[hwware]

Thanks @hwware! LGTM overall. Can you add some tests too?

Sure, ready to work, Thanks

[PingXie]

Thinking about this more, I feel that it is more preferable to model this new setting as a "soft" maxmemory, which can trigger key eviction earlier but won't cause OOM to be returned before hitting the actual maxmemory. Otherwise, we effectively create an alias of maxmemory. More specifically, I think performEviction should only return EVICT_FAIL when the memory usage goes beyond the real maxmemory.

Additionally, since getMaxmemoryState is also used outside of the OOM prevention path such as in VM_GetUsedMemoryRatio, we should consider parameterizing maxmemory and having it passed in by the caller instead, so that we can maintain the same semantics in these externally facing scenarios.

Thoughts?

[hwware]

Like what you suggested, if we can think when "maxmemory_available" is available, it is a soft maxmemory. Then you maybe think we should not return OOM, in the case, how we can return message to client, any idea?

[PingXie]

right - how about just processing the command normally?

1. if used_memory is below soft max, no change to the existing logic
2. if used_memory is above soft max but below hard max, trigger key eviction and continue with the normal processing
3. if used_memory is above hard max, no change to the existing logic, i.e., trigger key eviction and fail the command if the used_memory is still above hard max after the eviction)

[hwware]

@PingXie According to your last comment suggestion, I made some code changes as below:

1. In getMaxmemoryState, I update the level calculation way to *level = (float)mem_used / (float)server.maxmemory; instead of pass the maxmemory as a function parameter

2. performEvictions return EVICT_FAIL only when the memory usage goes beyond the real maxmemory (server.maxmemory)

3. if used_memory is above soft max but below hard max, trigger key eviction and continue with the normal processing

Please take a look, Thanks

[PingXie]

The main logic LGTM. Thanks @hwware!

[PingXie]

nit - one empty line?

Suggested change

[PingXie]

I don't think we should check server.maxmemory_reserved_scale and server.maxmemory_available. Also we should not goto cant_free on L725/728; instead, we should just break out of the while loop there and then always compute the result based on the current memory state. Otherwise, failing to evict keys due to used memory being greater than the new "soft" limit will always result in EVICT_FAIL/OOM rejection.

Suggested change

	if (server.maxmemory_reserved_scale && server.maxmemory_available) {
	size_t mem_used = zmalloc_used_memory();
	size_t overhead = freeMemoryGetNotCountedMemory();
	mem_used = (mem_used > overhead) ? mem_used - overhead : 0;
	if (mem_used > server.maxmemory) {
	result = EVICT_FAIL;
	goto cant_free;
	}
	}
	size_t mem_used = zmalloc_used_memory();
	size_t overhead = freeMemoryGetNotCountedMemory();
	mem_used = (mem_used > overhead) ? mem_used - overhead : 0;
	if (mem_used > server.maxmemory) {
	result = EVICT_FAIL;
	goto cant_free;
	}

[hwware]

Hi Ping, I think you misunderstand the logic for this condition: server.maxmemory_reserved_scale && server.maxmemory_soft. Let me explain to you here:
There are 2 cases:
Case 1. server.maxmemory_soft == server.maxmemory.
In this case, if getMaxmemoryState() return C_ERR, server should make the key eviction. then in the following for... loop, some keys are evicted and some memory are released. Basically, we can guarantee used_memory is below soft max. We do not need to calculate the used_memory again and just keep current logic.

Case 2. server.maxmemory_soft < server.maxmemory.
In this case, even we make some key eviction behavior, but we are not sure if used_memory is smaller than the soft memory, then we should calculate here.

Thus, only when server.maxmemory_soft_scale is set, we need to recalculate the used_memory （I think keep one condition if (server.maxmemory_soft_scale) is enough because server.maxmemory_soft is always true）

[PingXie]

With the recent update, we are essentially introducing a new "soft" version of "maxmemory". What do you think about the following?

Suggested change

	#
	# This parameter allows Valkey to evict keys earlier. The value represents
	# a percentage of the maxmemory limit, indicating how much memory the Valkey instance reserves
	# without storing data.
	# The default is 0, and the value can be set between 10 and 60.
	#
	# maxmemory-reserved-scale 0
	#
	# `maxmemory-soft-scale` defines a "soft" memory threshold as a scale of the `maxmemory` limit.
	# When memory usage exceeds this scale, Valkey begins proactive key eviction. However, exceeding this
	# threshold does not immediately reject new write commands; only the hard `maxmemory` limit will do so.
	# This scale is specified as a percentage of `maxmemory`, with a valid range between 10 and 60.
	#
	# maxmemory-soft-scale 0

[hwware]

Addressed. Parameter name, variable name, and function name had been changed upon request.

[PingXie]

If we are aligned on the config name maxmemory_soft_scale, I would suggest going with maxmemory_soft for the field name too

Suggested change

	server.maxmemory_available = (unsigned long long)server.maxmemory / 100.0 * (100 - server.maxmemory_reserved_scale);
	server.maxmemory_soft = (unsigned long long)server.maxmemory / 100.0 * (100 - server.maxmemory_soft_scale);