fix(server) : dont apply eviction on rss over limit #4276
Conversation
Signed-off-by: adi_holden <adi@dragonflydb.io>
tests/dragonfly/memory_test.py
Outdated
| @@ -28,6 +28,9 @@ async def calculate_estimated_connection_memory( | |||
| for conn in connections: | |||
| await conn.close() | |||
|
|
|||
| logging.info( | |||
| f"Estimated connection memory: {estimated_connections_memory // connections_number}." | |||
| ) | |||
| return estimated_connections_memory // connections_number | |||
There was a problem hiding this comment.
This method can be removed
| f'Current rss: {memory_info["used_memory_rss"]}. rss eviction threshold: {rss_max_memory * 0.9}.' | ||
| ) | ||
| stats_info = await async_client.info("stats") | ||
| logging.info(f'Current evicted: {stats_info["evicted_keys"]}. Total keys: {num_keys}.') |
There was a problem hiding this comment.
We can add an assertion after this loop to verify that at least 5% of the num_keys have been evicted
|
I have a question: could we try to free some memory when we detect that the RSS memory usage significantly differs from the basic memory usage? I’m not entirely sure where this logic should be implemented, but I believe it’s worth to add this. |
| # Test that used memory is less than 90% of max memory | ||
| memory_info = await async_client.info("memory") | ||
| assert ( | ||
| memory_info["used_memory"] < max_memory * 0.9 | ||
| ), "Used memory should be less than 90% of max memory." | ||
| assert ( | ||
| memory_info["used_memory_rss"] < rss_max_memory * 0.9 | ||
| memory_info["used_memory_rss"] > rss_max_memory * 0.9 | ||
| ), "RSS memory should be less than 90% of rss max memory (max_memory * rss_oom_deny_ratio)." |
There was a problem hiding this comment.
nit.: "RSS memory should be higher than 90% of rss max memory (max_memory * rss_oom_deny_ratio)."
Signed-off-by: adi_holden <adi@dragonflydb.io>
adiholden
changed the title
We should discuss how to improve the logic , I disabled it for now, see my updated description comment. |
This PR disables the eviction due to rss over limit
The problem with current eviction logic when rss is above the rss threshold is that we evict to many keys to reduce the rss usage.
In the test that I changed now we just debug populate a little bit higher than rss max memory threshold
If you run the test you can see that the eviction kicks in but continues for long time as we evict items but rss does not decrease
in this test we create 754974 keys and eventually get to evict 307100 keys to just drop 200MB of rss memory, while the the total rss memory is 4G.
So we are evicting so many items to get below the rss threshold (decrease 200MB), and this is because deleting items does not immediately decrease rss but only when defrag logic kicks in and starts to defragment the memory and calls decommit we see decrease in rss.
We need to improve the logic before enabling it