MEMORY DOCTOR/MALLOC-STATS in loading phase

[knggk]

The problem/use-case that the feature addresses

Sometimes I wish to investigate memory issues during the loading phase. However I get:

socket_redis> memory malloc-stats
LOADING Redis is loading the dataset in memory

Description of the feature

I would like to see the memory details provided by these commands during loading

Additional information

The check seems to be here https://github.com/valkey-io/valkey/blob/unstable/src/server.c#L4176-L4181:

    /* Loading DB? Return an error if the command has not the
     * CMD_LOADING flag. */
    if (server.loading && !server.async_loading && is_denyloading_command) {
        rejectCommand(c, shared.loadingerr);
        return C_OK;
    }

The question: Is there specific reasons those commands are disallowed during loading?

If not, is it as simple as adding CMD_LOADING flag to all the MEMORY commands, or there are considerations to take into account for making this work?

[hwware]

The command with flag "LOADING" indicates that this command has no any interaction with the data set. During loading process, data is written into memory, thus memory malloc-stats and related commands are not allowed to run.

[knggk]

@hwware Understood about LOADING being defined as a flag for commands not interacting with the dataset. Apart from that definition, is it not a potential valid use case to dump malloc-stats for investigation during loading?

[madolson]

I think we can allow this, it shouldn't be a breaking change, @knggk do you want to open a PR? We might also want to take a look at similar commands to see which ones we should enable so we stop flipping them one at a time :)

[knggk]

@madolson

We might also want to take a look at similar commands to see which ones we should enable so we stop flipping them one at a time :)

Agree...I was thinking doing all memory commands.

One additional question: any pointers on how to unit test this assuming we want to?

[hpatro]

One additional question: any pointers on how to unit test this assuming we want to?

@knggk I think it's fine to write a tcl test and slowdown the rdb loading via key-load-delay config and then perform the MEMORY * command operation.

[knggk]

I will look into TCL + key-load-delay. Thanks @hpatro!