INFO report peak memory before eviction#7894
INFO report peak memory before eviction#7894oranagra merged 1 commit intoredis:unstablefrom oranagra:peak_momentary_mem
Conversation
|
@redis/core-team please let me know how you feel about this. |
|
I think it's confusing. We know that stat_peak_memory is sampled, but all the documentation indicates it's the "max memory consumed in bytes by Redis." So there is a peak, but also a higher peak? I would almost be okay with just updating stat_peak_memory so it included these momentary peaks. |
|
@madolson that's what i was about to do (make sure to update the peak in other places), but then i realized that having one that doesn't include momentary spikes is beneficial too. that said, i'm still hesitant, made the PR to get feedback more than an attempt to push it. another alternative is to have the old would like to get more feedback. |
|
@oranagra I can't think of a case where the steady state gives you that much more information. I would prefer to just update the used_memory_peak, but I adding the steady state metric seems fine, it's basically free. |
oranagra
requested review from
madolson and
yossigo
and removed request for
yossigo
In some cases one command added a very big bulk of memory, and this would be "resolved" by the eviction before the next command. Seeing an unexplained mass eviction we would wish to know the highest momentary usage too. Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB loading.
|
@yossigo @madolson wasn't sure if you mean i should keep the original measurement and just put it into a new info field, and have the new measurement in the old info field. |
oranagra
changed the title
|
@oranagra What I don't like about the "steady" peak is that it's not really steady, it's just a sampled value taken in an arbitrary (cron) interval. |
|
@yossigo so if do we want it, maybe we can try to come up with other places to sample it. if we can't come up with such a thing, then let's take what's currently in this PR, and have just one true peak. |
|
@oranagra I think we should take it as it is, the way I see it that's a bug fix. |
In some cases one command added a very big bulk of memory, and this would be "resolved" by the eviction before the next command. Seeing an unexplained mass eviction we would wish to know the highest momentary usage too. Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB loading. The fix in clientsCronTrackExpansiveClients is related to #7874 (cherry picked from commit 457b707)
In some cases one command added a very big bulk of memory, and this would be "resolved" by the eviction before the next command. Seeing an unexplained mass eviction we would wish to know the highest momentary usage too. Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB loading. The fix in clientsCronTrackExpansiveClients is related to redis#7874
In some cases one command added a very big bulk of memory, and this would be "resolved" by the eviction before the next command. Seeing an unexplained mass eviction we would wish to know the highest momentary usage too. Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB loading. The fix in clientsCronTrackExpansiveClients is related to redis#7874 (cherry picked from commit 457b707)
3 participants
In some cases one command added a very big bulk of memory, and this
would be "resolved" by the eviction before the next command.
The current peak memory which is only being tracked in serverCron, which
may be useful to see what was the highest steady state the server was
facing.
But in some cases seeing an unexplained mass eviction we would wish to
know the highest momentary usage too.
Tracking it in call() and beforeSleep() adds some hooks in AOF and RDB
loading.
The fix in clientsCronTrackExpansiveClients is related to #7874