Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.75%. Comparing base (73838a0) to head (0975a7c).
⚠️ Report is 40 commits behind head on unstable.

@@             Coverage Diff              @@
##           unstable    #1840      +/-   ##
============================================
+ Coverage     74.69%   74.75%   +0.05%     
============================================
  Files           129      129              
  Lines         71199    71199              
============================================
+ Hits          53185    53222      +37     
+ Misses        18014    17977      -37     

I am not sure if 100% as default value is reasonable

We discussed this briefly before we released 8.0. Here is the discussion: #653 (comment).

My thoughts:

• Clients using 100% of the configured maxmemory is very unlikely. If it happens, then we have other serious problems: It means all keys are being evicted or are already evicted, or if key eviction is disabled, we are already far over maxmemory and clients receive the OOM error.
• I'm concerned about the performance degradation raised by @hpatro.
• A better value would be 50% or 10% but we can't use that as the default value because it would be a breaking change. I think users who want client eviction need to configure it themselves.

We discussed this briefly before we released 8.0. Here is the discussion: #653 (comment).

My thoughts:

• Clients using 100% of the configured maxmemory is very unlikely. If it happens, then we have other serious problems: It means all keys are being evicted or are already evicted, or if key eviction is disable, we are already far over maxmemory and clients receive the OOM error.
• I'm concerned about the performance degradation raised by @hpatro.
• A better value would be 50% or 10% but we can use that as the default value because it would be a breaking change. I think users who want client eviction need to configure it themselves.

I prefer to leave it and let client decide the default value.

Consensus from our weekly meeting was that we aren't entirely clear what problem this is trying to solve. There were concerns about folks migrating from Redis seeing different performance. So the preference was to not change the default and let users decide. This seems aligned with what other folks have said in this thread.

@hpatro @zuiderkwast

I revert the change and only add the comment to mention the performance issue, please check.