use startEvictionTimeProc() in config set maxmemory#10019
use startEvictionTimeProc() in config set maxmemory#10019oranagra merged 3 commits intoredis:unstablefrom
Conversation
soloestoy
force-pushed
the
prop-opt-1-config-set-maxmemory
branch
from
54261c5 to
c889245
Compare
We support incremental eviction now, and eviction should only be performed before a command execution (in processCommand()) or in the time event proc. Eviction should be forbidden during a command execution, see redis#9890 and redis#10014.
soloestoy
force-pushed
the
prop-opt-1-config-set-maxmemory
branch
from
c889245 to
ab3e30b
Compare
oranagra
left a comment
oranagra
left a comment
There was a problem hiding this comment.
in the past, i was hesitant to do that
i.e. rely on timers (cron / beforeSleep, future commands) to do that.
the reason was that i thought the caller may be issuing an INFO MEMORY, right after the CONFIG SET, and he expects to see the difference being applied.
this could be our tests, but also some actual users.
These days with the incremental eviction, i suppose this change doesn't break any concern that's not already broken, since anyway, the caller can't rely on the entire memory overhead to be evicted right away.
i.e. it is likely that the config change is a drastic one (e.g. from 5gb to 4gb), and we'll exit the eviction loop early.
unless of course, the user (or test) disabled the incremental eviction (i.e. maxmemory-eviction-tenacity).
i.e. some users or tests may have got bugs when this incremental eviction thing was introduced, and disabled it in order to use the new version of redis without solving this issue (now we break them)
|
for the record, we discussed this in a core-team meeting.
this should also apply for |
|
on a second thought, maybe it's a better idea to handle |
oranagra
added
release-notes
|
@redis/core-team please approve |
oranagra
added
the
breaking-change
4 participants
This would mean that the effects of
CONFIG SET maxmemorymay not be visible once the command returns.That could anyway happen since incremental eviction was added in redis 6.2 (see #7653)
We do this to fix one of the propagation bugs about eviction see #9890 and #10014.