The slave-serve-stale-data config option is useful, but I actually would like a mode where a slave that has succesfully loaded data at least once to be allowed to serve stale data, but a new slave that has not yet loaded any data should not serve the empty database

This is a real issue for servers when the RDB is in the multiple gigabytes, as it can take many minutes to save, transfer, and load the database. During the period when the server is saving and transferring the data, I only have the option of not allowing stale data or incorrectly serving empty results which can't be distinguished from truly missing keys.

I've implemented a proof of concept of this at https://github.com/cxreg/redis/commit/2a5236952c5658a887b3aa05fa1e949b907f1e78. It basically just duplicates what slave-serve-stale-data does but with a server.has_loaded_data_at_least_once flag

How does this sound?

Comment From: yoav-steinberg

I think this makes sense. @oranagra ?

Comment From: oranagra

the concept sounds good to me (the implementation should probably be different). maybe instead of adding a new config, we can just change the implementation behind the existing stale data serving, to avoid serving when there's no data. @yossigo @madolson WDYT? @eduardobr this is somewhat in the area of #9323

Comment From: eduardobr

I think it differs in the sense that #9323 applies to replications where db is already up and running and here it's about startup, but I'm confused about "SYNC with master in progress". It's not in code. Is it actually wrong documentation and meant to be the "MASTERDOWN Link with MASTER is down and replica-serve-stale-data is set to 'no'"?

By my experience, the replica initialization is pretty hard to probe because it starts returning empty data (or successful PING), then enters LOADING mode, then successful again. On K8s side we need to add artificial delays in startup probes and usually consider ready later than needed just to be safe. So this is a really good addition for proper startup probes. @kuznero

Now looking broadly the name of this configuration, I remember my eyes lit up when I found it because I thought I would be able to serve stale data during LOADING, but it has only been about disconnections with master, and not about the period where LOADING is happening. @oranagra But this is something else and falls into my suggestion that the whole thing about the new SWAPDB could be turned on for disk-based replication as well, as long as the user accepts the temporary high memory consumption by setting some flag.

Comment From: oranagra

@eduardobr note that this is an old one, so maybe error message was different. also regarding fast startups, there are two recent changes that make it very fast. 1. the diskless replication means it'l enter LOADING quite fast, but in disk-based it'll only happen after it finished transferring the rdb file. 2. in redis 6.2 i've added a few things to speed up the initial connection and re-connection attempts to be immediate rather than after 1 second delay.

regarding enabling the "swapdb" feature even on disk-based, i must say i don't like it.. i never really liked that whole "swapdb" feature, and i kinda regret adding it (did so at the request for Salvatore). the reason i accepted your suggestion of allowing reads during that loading was that if we already have that feature, there's no reason not to benefit from it's "side effects".

anyway, bottom line, regarding the suggestion in this issue (blocking stale reads on an empty replica), do you see any reason not to do that by default? i.e. instead of adding a new config, we should modify the behavior of slave-serve-stale-data not to serve when there's no previous data.

Comment From: eduardobr

@oranagra, actually documentation is still not matching the real message (already reported in https://github.com/redis/redis/issues/3131) I think it's great to have default behavior of slave-serve-stale-data not to serve when there's no previous data (but of course start serving if it synced successfully with a master that has no data). Will just be a bit strange (even though not totally wrong) that the startup message will be MASTERDOWN Link with MASTER is down

Comment From: eduardobr

@oranagra Btw:

    /* We don't allow PING during loading since in Redis PING is used as
     * failure detection, and a loading server is considered to be
     * not available. */
    {"ping",pingCommand,-1,
     "ok-stale fast sentinel @connection"},

Ping will fail on Loading but succeed when master is down and replica-serve-stale-data=no I know that it would be a breaking change, but current behavior seems to need a fix. Considering message in redis.conf was wrong, maybe an acceptable change for Redis 7.

I've opened an issue in bitnami redis chart to take in consideration the MASTERDOWN status in readiness probe, but that will be hard as some other dummy command will have to be used instead of PING: https://github.com/bitnami/charts/issues/8016

Comment From: ShooterIT

Hi @oranagra @eduardobr Similar with #9287 if replicas never synced with master, we don't think it can serve stale data? WDYT?

if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED &&
    (server.repl_serve_stale_data == 0 || replicationGetSlaveOffset() == 0) &&
    is_denystale_command)
{
    rejectCommand(c, shared.masterdownerr);
    return C_OK;
}

Comment From: oranagra

@ShooterIT seem right to me, as i stated before, i don't think a config flag is needed. please submit a PR and let's get it approved.

@eduardobr are you suggesting to remove the ok-stale flag from the PING command?

Comment From: eduardobr

@oranagra from the point of view where it must be consistent with how it behaves when LOADING, yes.

But if that makes things very complicated, we may need to think in kubernetes world and make a simple ISREADY and a ISALIVE command.

ISALIVE would be like PING but always return success doesn’t matter what (even when LOADING)

ISREADY would return success only if it is ready to serve data (so no ok-stale and no ok-loading) Edit: also solves how hard it is to probe startup (because until it becomes ready there are moments of successful PING, then LOADING, then success again). ISREADY could be successful only when master_link_status=up. This would be very handy and clean for any system checking status of redis.

WDYT?

Comment From: yossigo

Discussed this briefly with @oranagra. Clients that use PING to only check connectivity should anyway be ready to receive different errors regarding the server state (i.e. -NOAUTH, -LOADING, -BUSY) so it's reasonable to also return -MASTERDOWN even if it breaks backwards compatibility.

Comment From: oranagra

i suppose we better handle that in a separate PR (clearer release notes). @eduardobr please open one.

Comment From: oranagra

had another look at this issue. i think that if the slave is empty (either never synced with the master yet), or it flushed it's data for a diskless sync which failed, it should respond with MASTERDOWN (even if replica-serve-stale-data==yes).

while on that subject, i noticed that when a disk-based sync rdbLoad fails, we don't call emptyData. this is a rare event, but i think we should fix it, and either disk-based or diskless loading fails and we end up with an empty data, make sure that we don't serve any commands, regardless of replica-serve-stale-data setting.

i'm adding this issue to 8.0 tasks.

Comment From: eduardobr

or it flushed it's data for a diskless sync which failed

We don’t have this kind of behavior anymore for diskless sync. Flushing current db happens only after successful replication.

Comment From: oranagra

only in the swapdb mode, in the other mode, in the on-empty-db it flushes the half baked data. anyway, point is that if the replica doesn't have any (full set of) data it got from a master (not based on the keyspace size, but rather on the replication offset), it should respond with MASTERDOWN, regardless of the stale data config.