Added explanation of maxmemory for replication.

[nastena1606]

Added how to monitor eviction

The change closes #166

[zuiderkwast]

Looks good to me.

@Redoubts Please take a look!

[Redoubts]

is this correct tho? used_memory appears to include replication buffers and the like, whereas used_memory_dataset seems to account for the "output buffers needed to feed the replicas are subtracted from the used memory count" part.

[zuiderkwast]

@Redoubts Are you happy with the last updates?

To answer you question in the issue:

In particular, if I'm looking to track how close I am to eviction, which field in INFO memory should I be tracking? I think used_memory follows and includes replication buffers, so is used_memory_dataset sufficient to track instead? And what about queries that use a lot of memory, do they impact anything?

Eviction is triggered by used_memory reaching the configured maxmemory. This means that increased memory usage by other things, such as client buffers, can cause keys to be evicted.

Should we update the text in the config file too in some way?

[Redoubts]

Eviction is triggered by used_memory reaching the configured maxmemory. This means that increased memory usage by other things, such as client buffers, can cause keys to be evicted.

I guess that's surprising to hear, since when I asked a similar question to the Redis project, they included:

The memory used by this buffer is not included in the total that is compared to maxmemory to see if eviction is required.

https://github.com/redis/docs/pull/821/files#diff-fc7b78d56dfacf469e9614adc8a4e80baecb6e2cf42367b12728a67956ccdb7cR66-R72

is this a documented difference from Redis, or am I misunderstanding something?

[zuiderkwast]

Ohh... TBH, I'm not really sure and our docs don't say... I guess the best way to be sure to go to the source code. It appears that you (and Redis) are right.

This is the function that computes how much memory needs to be freed by eviction, getMaxmemoryState: https://github.com/valkey-io/valkey/blob/unstable/src/evict.c#L380.

And this function computes the memory to be excluded from the total used memory, the AOF and replica output buffers, freeMemoryGetNotCountedMemory: https://github.com/valkey-io/valkey/blob/unstable/src/evict.c#L320

So the text in the valkey.conf is correct.

# WARNING: If you have replicas attached to an instance with maxmemory on,
# the size of the output buffers needed to feed the replicas are subtracted
# from the used memory count, so that network problems / resyncs will
# not trigger a loop where keys are evicted, and in turn the output
# buffer of replicas is full with DELs of keys evicted triggering the deletion
# of more keys, and so forth until the database is completely emptied.

Now, your question is if there is any field in INFO that can show this subtracted memory size. I don't know. @enjoy-binbin, @madolson do you know?

If there is no such field, should we add one?

[enjoy-binbin]

we do have a mem_not_counted_for_evict in the INFO command, this include the replication buffer and aof buffer.

[zuiderkwast]

we do have a mem_not_counted_for_evict in the INFO command, this include the replication buffer and aof buffer.

Thanks @enjoy-binbin! So we can describe the condition for eviction with this formula of configs and info fields?

if (used_memory - mem_not_counted_for_evict > maxmemory) evict some keys ...

[enjoy-binbin]

yes, that is updateMaxmemory warning:

static int updateMaxmemory(const char **err) {
    UNUSED(err);
    if (server.maxmemory) {
        size_t used = zmalloc_used_memory() - freeMemoryGetNotCountedMemory();
        if (server.maxmemory < used) {
            serverLog(LL_WARNING,
                      "WARNING: the new maxmemory value set via CONFIG SET (%llu) is smaller than the current memory "
                      "usage (%zu). This will result in key eviction and/or the inability to accept new write commands "
                      "depending on the maxmemory-policy.",
                      server.maxmemory, used);
        }
        startEvictionTimeProc();
    }
    return 1;
}

Better understanding

[zuiderkwast]

@nastena1606 Can you involve mem_not_counted_for_evict in this text?

[zuiderkwast]

Thanks @nastena1606! This looks great.