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

hwware · 2024-07-26T15:24:25Z

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 · 2024-07-26T15:53:55Z

Codecov Report

Attention: Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.72%. Comparing base (a62d1f1) to head (4da32a2).

Files with missing lines	Patch %	Lines
src/config.c	80.00%	2 Missing ⚠️
src/evict.c	85.71%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable     #831      +/-   ##
============================================
+ Coverage     70.65%   70.72%   +0.06%     
============================================
  Files           114      114              
  Lines         61799    63091    +1292     
============================================
+ Hits          43664    44619     +955     
- Misses        18135    18472     +337

Files with missing lines	Coverage Δ
src/server.c	`88.79% <100.00%> (+0.15%)`	⬆️
src/server.h	`100.00% <100.00%> (ø)`
src/evict.c	`96.64% <85.71%> (-1.09%)`	⬇️
src/config.c	`78.85% <80.00%> (+0.15%)`	⬆️

... and 89 files with indirect coverage changes

enjoy-binbin · 2024-09-11T15:14:45Z

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

madolson · 2024-09-11T15:59:51Z

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 · 2024-09-25T04:11:28Z

src/evict.c

+    } else if (mem_reported <= server.maxmemory && !level)
+        return C_OK;


nit. add {} to match the if part?

Suggested change

} else if (mem_reported <= server.maxmemory && !level)

return C_OK;

} else if (mem_reported <= server.maxmemory && !level) {

return C_OK;

}

PingXie · 2024-09-25T04:13:44Z

src/server.c

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


extract this formula to an inline function? this has been repeated 3 times

Address. I just create an inline function in server.h

PingXie · 2024-09-25T04:15:24Z

src/evict.c

+    if (server.maxmemory_reserved_scale) {
+        mem_tofree = mem_used - server.maxmemory_available;
+    } else
+        mem_tofree = mem_used - server.maxmemory;


is this check needed? if server.maxmemory_reserved_scale is 0, server.maxmemory_available would be equal to server.maxmemory.

similarly, this would apply to the other changes in this function.

PingXie · 2024-09-25T04:31:26Z

+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?

src/evict.c

PingXie · 2024-09-29T20:28:08Z

src/server.h

@@ -3010,6 +3012,9 @@ void trimStringObjectIfNeeded(robj *o, int trim_small_values);
 static inline int canUseSharedObject(void) {
    return server.maxmemory == 0 || !(server.maxmemory_policy & MAXMEMORY_FLAG_NO_SHARED_INTEGERS);
 }
+static inline void calculateMaxAvailableMemory(void) {


Since we are not returning any value from this function but updating a state in server, I feel that `update would be a better verb to indicate that this is more of a "method" than a "function".

Suggested change

static inline void calculateMaxAvailableMemory(void) {

static inline void updateMaxAvailableMemory(void) {

hwware · 2024-10-02T13:52:11Z

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

Sure, ready to work, Thanks

src/config.c

valkey.conf

PingXie · 2024-10-11T06:23:53Z

src/evict.c

@@ -398,11 +398,12 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev
    if (total) *total = mem_reported;

    /* We may return ASAP if there is no need to compute the level. */
-    if (!server.maxmemory) {
+    if (!server.maxmemory_available) {


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?

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?

right - how about just processing the command normally?

if used_memory is below soft max, no change to the existing logic

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

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)

Signed-off-by: hwware <[email protected]>

hwware force-pushed the maxmemory-reserved-parameter branch from e31dd8b to b8a98df Compare July 26, 2024 15:39

hwware requested review from PingXie, enjoy-binbin and madolson and removed request for PingXie and enjoy-binbin July 26, 2024 15:52

hwware force-pushed the maxmemory-reserved-parameter branch from b8a98df to ba13a0c Compare August 29, 2024 14:50

hwware force-pushed the maxmemory-reserved-parameter branch from ba13a0c to e27e9de Compare September 9, 2024 16:16

hwware force-pushed the maxmemory-reserved-parameter branch from e27e9de to 0a68013 Compare September 16, 2024 17:10

hwware force-pushed the maxmemory-reserved-parameter branch from 0a68013 to 9596c78 Compare September 24, 2024 01:14

PingXie reviewed Sep 25, 2024

View reviewed changes

hwware force-pushed the maxmemory-reserved-parameter branch 2 times, most recently from 35437eb to db966fe Compare September 26, 2024 02:46

PingXie reviewed Sep 29, 2024

View reviewed changes

hwware force-pushed the maxmemory-reserved-parameter branch 4 times, most recently from f7cc8c8 to 7a5584b Compare October 7, 2024 09:31

PingXie reviewed Oct 11, 2024

View reviewed changes

hwware force-pushed the maxmemory-reserved-parameter branch 3 times, most recently from 200b203 to 4da32a2 Compare October 18, 2024 05:54

hwware force-pushed the maxmemory-reserved-parameter branch from 4da32a2 to 510265f Compare October 28, 2024 13:42

hwware added 7 commits October 28, 2024 13:42

Add maxmemory-reserved-scale parameter for evicting key earlier

4219073

Signed-off-by: hwware <[email protected]>

clang-format adjust

74d0eb2

Signed-off-by: hwware <[email protected]>

Rebase and address Ping commnt

765c0ab

Signed-off-by: hwware <[email protected]>

adjust clang-format

1813987

Signed-off-by: hwware <[email protected]>

Add test case and Address Ping Comment

59892a5

Signed-off-by: hwware <[email protected]>

Update comments

7e9fa70

Signed-off-by: hwware <[email protected]>

Update level but test case fail

c6db98b

Signed-off-by: hwware <[email protected]>

hwware force-pushed the maxmemory-reserved-parameter branch from 510265f to c6db98b Compare October 28, 2024 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hwware commented Jul 26, 2024

codecov bot commented Jul 26, 2024 •

edited

Loading

enjoy-binbin commented Sep 11, 2024

madolson commented Sep 11, 2024

PingXie left a comment

PingXie Sep 25, 2024

hwware Sep 26, 2024

PingXie Sep 25, 2024

hwware Sep 26, 2024

PingXie Sep 25, 2024

hwware Sep 26, 2024

PingXie commented Sep 25, 2024

PingXie left a comment

PingXie Sep 29, 2024 •

edited

Loading

hwware Oct 11, 2024

hwware commented Oct 2, 2024

PingXie Oct 11, 2024

hwware Oct 18, 2024 •

edited

Loading

PingXie Oct 24, 2024

		} else if (mem_reported <= server.maxmemory && !level)
		return C_OK;

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

	static inline void calculateMaxAvailableMemory(void) {
	static inline void updateMaxAvailableMemory(void) {

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

Are you sure you want to change the base?

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

Conversation

hwware commented Jul 26, 2024

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

enjoy-binbin commented Sep 11, 2024

madolson commented Sep 11, 2024

PingXie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PingXie commented Sep 25, 2024

PingXie left a comment

Choose a reason for hiding this comment

PingXie Sep 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwware commented Oct 2, 2024

Choose a reason for hiding this comment

hwware Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 26, 2024 •

edited

Loading

PingXie Sep 29, 2024 •

edited

Loading

hwware Oct 18, 2024 •

edited

Loading