Relaxed RDB-version check in RDB load

Signed-off-by: Viktor Söderqvist <[email protected]>

src/rdb.c

@@ -42,6 +42,7 @@
 #include <math.h>
 #include <fcntl.h>
 #include <stdatomic.h>
+#include <stdbool.h>
 #include <sys/types.h>
 #include <sys/time.h>
 #include <sys/resource.h>
@@ -1418,6 +1419,7 @@ int rdbSaveRio(int req, rio *rdb, int *error, int rdbflags, rdbSaveInfo *rsi) {
     int j;
 
     if (server.rdb_checksum) rdb->update_cksum = rioGenericUpdateChecksum;
+    /* TODO: Change this to "VALKEY%03d" next time we bumb the RDB version. */
     snprintf(magic, sizeof(magic), "REDIS%04d", RDB_VERSION);
     if (rdbWriteRaw(rdb, magic, 9) == -1) goto werr;
     if (rdbSaveInfoAuxFields(rdb, rdbflags, rsi) == -1) goto werr;
@@ -3023,17 +3025,24 @@ int rdbLoadRioWithLoadingCtx(rio *rdb, int rdbflags, rdbSaveInfo *rsi, rdbLoadin
     char buf[1024];
     int error;
     long long empty_keys_skipped = 0;
+    bool is_valkey_magic;
 
     rdb->update_cksum = rdbLoadProgressCallback;
     rdb->max_processing_chunk = server.loading_process_events_interval_bytes;
     if (rioRead(rdb, buf, 9) == 0) goto eoferr;
     buf[9] = '\0';
-    if (memcmp(buf, "REDIS", 5) != 0) {
+    if (memcmp(buf, "REDIS0", 6) == 0) {
+        is_valkey_magic = false;
+    } else if (memcmp(buf, "VALKEY", 6) == 0) {
+        is_valkey_magic = true;
+    } else {
         serverLog(LL_WARNING, "Wrong signature trying to load DB from file");
         return C_ERR;
     }
-    rdbver = atoi(buf + 5);
-    if (rdbver < 1 || rdbver > RDB_VERSION) {
+    rdbver = atoi(buf + 6);
+    if (rdbver < 1 ||
+        (rdbver >= RDB_FOREIGN_VERSION_MIN && !is_valkey_magic) ||
+        (rdbver > RDB_VERSION && server.rdb_version_check == RDB_VERSION_CHECK_STRICT)) {
         serverLog(LL_WARNING, "Can't handle RDB format version %d", rdbver);
         return C_ERR;
     }

src/rdb.h

@@ -37,9 +37,27 @@
 #include "server.h"
 
 /* The current RDB version. When the format changes in a way that is no longer
- * backward compatible this number gets incremented. */
+ * backward compatible this number gets incremented.
+ *
+ * RDB 11 is the last open-source Redis RDB version, used by Valkey 7.x and 8.x.
+ *
+ * RDB 12+ are non-open-source Redis formats.
+ *
+ * Next time we bump the Valkey RDB version, use much higher version to avoid
+ * collisions with non-OSS Redis RDB versions. For example, we could use RDB
+ * version 90 for Valkey 9.0.
+ *
+ * In an RDB file/stream, we also check the magic string REDIS or VALKEY but in
+ * the DUMP/RESTORE format, there is only the RDB version number and no magic
+ * string. */
 #define RDB_VERSION 11
 
+/* Reserved range for foreign (unsupported, non-OSS) RDB format. */
+#define RDB_FOREIGN_VERSION_MIN 12
+#define RDB_FOREIGN_VERSION_MAX 79
+static_assert(RDB_VERSION < RDB_FOREIGN_VERSION_MIN || RDB_VERSION > RDB_FOREIGN_VERSION_MAX,
+              "RDB version in foreign version range");
+
 /* Defines related to the dump file format. To store 32 bits lengths for short
  * keys requires a lot of space, so we check the most significant 2 bits of
  * the first byte to interpreter the length:

tests/integration/rdb.tcl

@@ -1,9 +1,21 @@
 tags {"rdb external:skip"} {
 
+# Helper function to start a server and kill it, just to check the error
+# logged.
+set defaults {}
+proc start_server_and_kill_it {overrides code} {
+    upvar defaults defaults srv srv server_path server_path
+    set config [concat $defaults $overrides]
+    set srv [start_server [list overrides $config keep_persistence true]]
+    uplevel 1 $code
+    kill_server $srv
+}
+
 set server_path [tmpdir "server.rdb-encoding-test"]
 
 # Copy RDB with different encodings in server path
 exec cp tests/assets/encodings.rdb $server_path
+exec cp tests/assets/encodings-rdb987.rdb $server_path
 exec cp tests/assets/list-quicklist.rdb $server_path
 
 start_server [list overrides [list "dir" $server_path "dbfilename" "list-quicklist.rdb" save ""]] {
@@ -15,11 +27,7 @@ start_server [list overrides [list "dir" $server_path "dbfilename" "list-quickli
     } {7}
 }
 
-start_server [list overrides [list "dir" $server_path "dbfilename" "encodings.rdb"]] {
-  test "RDB encoding loading test" {
-    r select 0
-    csvdump r
-  } {"0","compressible","string","aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+set csv_dump {"0","compressible","string","aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
 "0","hash","hash","a","1","aa","10","aaa","100","b","2","bb","20","bbb","200","c","3","cc","30","ccc","300","ddd","400","eee","5000000000",
 "0","hash_zipped","hash","a","1","b","2","c","3",
 "0","list","list","1","2","3","a","b","c","100000","6000000000","1","2","3","a","b","c","100000","6000000000","1","2","3","a","b","c","100000","6000000000",
@@ -33,6 +41,32 @@ start_server [list overrides [list "dir" $server_path "dbfilename" "encodings.rd
 "0","zset","zset","a","1","b","2","c","3","aa","10","bb","20","cc","30","aaa","100","bbb","200","ccc","300","aaaa","1000","cccc","123456789","bbbb","5000000000",
 "0","zset_zipped","zset","a","1","b","2","c","3",
 }
+
+start_server [list overrides [list "dir" $server_path "dbfilename" "encodings.rdb"]] {
+  test "RDB encoding loading test" {
+    r select 0
+    csvdump r
+  } $csv_dump
+}
+
+start_server_and_kill_it [list "dir" $server_path "dbfilename" "encodings-rdb987.rdb"] {
+    test "RDB future version loading, strict version check" {
+        wait_for_condition 50 100 {
+            [string match {*Fatal error loading*} \
+                 [exec tail -1 < [dict get $srv stdout]]]
+        } else {
+            fail "Server started even if RDB version check failed"
+        }
+    }
+}
+
+start_server [list overrides [list "dir" $server_path \
+                                  "dbfilename" "encodings-rdb987.rdb" \
+                                  "rdb-version-check" "relaxed"]] {
+    test "RDB future version loading, relaxed version check" {
+        r select 0
+        csvdump r
+    } $csv_dump
 }
 
 set server_path [tmpdir "server.rdb-startup-test"]
@@ -80,17 +114,6 @@ start_server [list overrides [list "dir" $server_path] keep_persistence true] {
     r del stream
 }
 
-# Helper function to start a server and kill it, just to check the error
-# logged.
-set defaults {}
-proc start_server_and_kill_it {overrides code} {
-    upvar defaults defaults srv srv server_path server_path
-    set config [concat $defaults $overrides]
-    set srv [start_server [list overrides $config keep_persistence true]]
-    uplevel 1 $code
-    kill_server $srv
-}
-
 # Make the RDB file unreadable
 file attributes [file join $server_path dump.rdb] -permissions 0222