Convert RDB to AOF on boot when appendonly on for the first time

[tmm1]

Fixes the issue in #837 where data loss will occur if you restart redis-server to turn appendonly on via redis.conf, without first calling redis-cli config set appendonly on on the running instance before restarting.

With this PR, redis-server will detect and perform the migration automatically on boot when a) appendonly on in the config (i.e. server.aof_state == REDIS_AOF_ON) and b) appendonly.aof does not exist on disk (i.e. redis_stat(server.aof_filename) == -1 && errno == ENOENT).

cc @antirez

[antirez]

Hello!

1. Aman cool to see you again among Redis PRs ;-) Thanks.
2. The provided solution sounds credible... cool.
3. Should we also do the reverse when the RDB file is missing but there is an AOF?

[tmm1]

Should we also do the reverse when the RDB file is missing but there is an AOF?

Maybe, but I think it makes sense for another PR. I think most users will want to upgrade from RDB (legacy) to AOF (new) so that's the case I wanted to optimize for. It's also much easier to deal with, because you can trigger it only when appendonly.aof is missing.

When switching back from AOF -> RDB, it is much more complicated because both files will exist and it's impossible to tell which one to prefer just based on timestamps. For instance if someone has both appendonly on and save 900 1 then both files will exist, and it's not clear which one should be preferred if the instance is booted with appendonly off.

I think the sanest approach is probably to print a WARNING or maybe even ERROR and exit if you boot redis with appendonly off but an appendonly.aof exists. This way there is no risk of data loss, and if the user wants to proceed they can delete or renamed the old appendonly.aof. If they realize the mistake, they can boot with appendonly on and create a fresh RDB first using BGSAVE or SAVE.

[antirez]

@tmm1 excellent suggestions, I agree. Thanks. Conceptually the feature is accepted, I'll review it in detail and will merge into unstable. If back porting is viable, I'll backport to 3.0 since it is an operational improvement.

[tmm1]

Great, thanks. FWIW we have backported this patch to 2.8 (applies cleanly as-is) and are using it successfully.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}