@soloestoy Ideally only changes of configuration should be denied during async_loading.
It's true there's an issue and missing test regarding CONFIG SET being allowed when it shouldn't.
But denying all commands the same way as when loading kinda defeats the purpose (which is also to allow serving data while doing the full sync).
Can you think about any other case that should be denied other than CONFIG SET?
What is the risk of EVAL?

Thanks

@eduardobr about eval, you can look deep into function processEventsWhileBlocked(), when rdb loading the rdbLoadProgressCallback callback would call it, and set the global variable ProcessingEventsWhileBlocked to 1, if we allow eval during rdb loading, and reach the lua_timeout, then the lua_timeout hook will step into processEventsWhileBlocked(), and then after lua timeout back to rdbLoadProgressCallback()'s processEventsWhileBlocked(), the global variable ProcessingEventsWhileBlocked is already set to 0.

I'm not sure if it has actual damage, but it's very dangerous, and maybe more commands are dangerous like module.

@soloestoy good catch about processEventsWhileBlocked collision, i'm ashamed that i missed it 8-).
i suppose we can solve it with a moderate effort.

Regarding config, i never really liked the fact it was blocked during loading (in the quire recent past, CONFIG GET was blocked too). there are only a handful configs that can be abused during loading, and i would argue that this abuse is unlikely to be a real problem for users.
however, since we identified that, let's solve that.

1. we should cache the config on the stack when entering the loading state, and avoid accessing the config for a second time.
2. let's see if there are any other problematic configs, and we can add an explicit check for async_loading in these to block them (or add a flag if there are many)

the configs that could have some possible issue are:

• appendonly
• repl-diskless-load
• sanitize-dump-payload (i actually don't care)
• loading-process-events-interval-bytes (don't see a real issue, maybe even an advantage)

apart from that, maybe we need to block some commands, specifically:

• REPLICAOF

@eduardobr @soloestoy can one of you take this issue and come up with a PR to solve these issues?
if not, maybe i'll try to find time to do that...

a easy fix is don't allow config set and eval/evalsha when async_loading, wdyt @oranagra

I don't mind blocking config set, but that one is actually easy to solve (forbid a few configs, and cache others on the stack).
Regarding EVAL, I think it would limit the usability of that feature, I rather solve it properly without blocking it, and I don't think it should be very hard.

ok, i understand no one else is looking into this, so i'll try to handle it myself.

Note that i just realized the case of nested calls to ProcessingEventsWhileBlocked already existed anyway, see: #8284 i.e. when a slow script is executed when loading AOF file.

And the fix for that seems as simple as incrementing / decrementing ProcessingEventsWhileBlocked instead of just setting it to 0 and 1.

handled in #9878

Closing in favor of #9878