bump the RDB version #2422

ranshid · 2025-08-04T18:39:01Z

This is needed due to changes presented in #2089

Signed-off-by: Ran Shidlansik <[email protected]>

codecov · 2025-08-04T18:54:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.57%. Comparing base (3b12132) to head (54a7d3f).
⚠️ Report is 17 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2422      +/-   ##
============================================
+ Coverage     71.51%   71.57%   +0.06%     
============================================
  Files           123      123              
  Lines         67454    67493      +39     
============================================
+ Hits          48239    48308      +69     
+ Misses        19215    19185      -30

Files with missing lines	Coverage Δ
src/aof.c	`80.53% <100.00%> (ø)`
src/rdb.c	`76.81% <100.00%> (-0.27%)`	⬇️
src/valkey-check-rdb.c	`72.11% <100.00%> (+0.33%)`	⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/rdb.h

src/aof.c

Signed-off-by: Ran Shidlansik <[email protected]>

src/valkey-check-rdb.c

Signed-off-by: Ran Shidlansik <[email protected]>

sarthakaggarwal97

nice!

Signed-off-by: Ran Shidlansik <[email protected]>

madolson · 2025-08-05T00:52:11Z

src/rdb.c

+        (rdbver >= RDB_FOREIGN_VERSION_MIN && rdbver <= RDB_FOREIGN_VERSION_MAX) ||
+        (rdbver > RDB_FOREIGN_VERSION_MAX && !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);


We should probably extend this to include whether or not it's the Redis magic as well?

@madolson to which one of the check conditions do you refer to? The [12-79] or just validate that below 12 we have the redis magic?

@madolson done. want to take a look?

Apparently not :D

I also meant we should print out whether or not the version is incompatible or it's the Redis Magic.

Since we don't reuse the same numbers as with the different magics, it will never be ambiguous (except for hand-hacked RDB files with wrong magic/number).

For us yes, not for end users. Ideally we should print out that this is a Redis RDB and not a valkey RDB.

Signed-off-by: Ran Shidlansik <[email protected]>

ranshid · 2025-08-05T06:14:40Z

@valkey-io/core-team just to highlight the implications of this: this basically breaks the 7.2 and 8.0 compatibility.
IIRC we only added the strict version check in 8.1 which will now become the only downgradable version.
Should we include some wording about it in the documentation? I expect it is going to be somewhat confusing.
Also, I expect some of the Cross version cluster tests will now fail. I think it is expected, but would be happy to hear if you feel otherwise.

ranshid · 2025-08-05T07:15:27Z

I think that the only issue is with the failover test.
This test will attempt to first sync-up an old version which is now unsupported. It will probably not be an issue for the daily compat tests as they are being run with compatible-redis tag.

I think that for the CI tests we can run only against 8.1 and enable the relaxed rdb check in this specific test.

@zuiderkwast WDYT?

zuiderkwast · 2025-08-05T08:18:26Z

IIRC we only added the strict version check in 8.1 which will now become the only downgradable version.

Ran, yes, because of this, we consider 8.1 a stepping stone. All our customers must upgrade to (a version of our products that contain) 8.1 before they can upgrade further to 9 and later.

I think that for the CI tests we can run only against 8.1 and enable the relaxed rdb check in this specific test.

Yes.

We have discussed a way to let Valkey produce older RDB versions for even better downgrade possibilities. It would involve a config to set the RDB version and it would be up to the user to only use features supported by the chosen RDB version. I still think we should do it. Technically, it's not very difficult... #1108.

ranshid · 2025-08-05T09:40:53Z

IIRC we only added the strict version check in 8.1 which will now become the only downgradable version.

Ran, yes, because of this, we consider 8.1 a stepping stone. All our customers must upgrade to (a version of our products that contain) 8.1 before they can upgrade further to 9 and later.

I think that for the CI tests we can run only against 8.1 and enable the relaxed rdb check in this specific test.

Yes.

We have discussed a way to let Valkey produce older RDB versions for even better downgrade possibilities. It would involve a config to set the RDB version and it would be up to the user to only use features supported by the chosen RDB version. I still think we should do it. Technically, it's not very difficult... #1108.

Thank you @zuiderkwast . I am mainly referring to the plan to change the CI as part of THIS pr so that we will start running the cross version compat against 8.1 + configure relaxed RDB check in the failover test

also make Cross version cluster - failover use relaxed rdb check Signed-off-by: Ran Shidlansik <[email protected]>

ranshid · 2025-08-05T11:11:15Z

hhh There is a failure in test Send cluster message via module from other server to latest, but it is because this test should have fail IMO. It is based on the fact that CONFIG resetstat will reset the cluster msg stats (which is not true). I actually think that there might be a bug in the use of light headers in this cross version case? will check

@hpatro FYI

ranshid · 2025-08-05T13:50:35Z

hhh There is a failure in test Send cluster message via module from other server to latest, but it is because this test should have fail IMO. It is based on the fact that CONFIG resetstat will reset the cluster msg stats (which is not true). I actually think that there might be a bug in the use of light headers in this cross version case? will check

@hpatro FYI

O.K after some fighting, I found the issue is known (#1708) and that is the node-id is passed NULL terminated only from 8.1 to clusterMassageRecievers. because of that the 7.2 module is unable to send the DONG message back to the originator and this is why the test fails for 7.2. So there are some AIs:

fix the failover test as it is now not testing correctly the message transfer.
@zuiderkwast / @hpatro seems like you guys have context on the issue. did we decide to backport this? if not what do we expect?
I guess after enabling this test to work with 8.1 there will be no failure though

…ages Signed-off-by: Ran Shidlansik <[email protected]>

tests/unit/moduleapi/cross-version-cluster.tcl

hpatro · 2025-08-05T18:50:18Z

src/rdb.h

 * 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.
+ * RDB 12-79 are reserved for Redis non-compatible RDB formats


Any particular reason to pick 79?

With Valkey 9.0 we could have picked 90 as our new RDB version :).

This was discussed in the last core-meeting and we decided on 80. Personally I think the 80's (although the bad hair thing) were so cool

80+ for Valkey magic was picked in #1604. It was arbitrary-ish.

Yeah we could have used 90 for Valkey 9.0 but with this scheme we would run out of RDB versions after Valkey 99.9. If we just bumb it when necessary, we don't run out of RDB numbers as fast.

This is needed due to changes presented in valkey-io#2089 --------- Signed-off-by: Ran Shidlansik <[email protected]>

bump the RDB version

cb58c73

Signed-off-by: Ran Shidlansik <[email protected]>

ranshid added this to Valkey 9.0 Aug 4, 2025

ranshid requested a review from zuiderkwast August 4, 2025 18:39

zuiderkwast reviewed Aug 4, 2025

View reviewed changes

src/rdb.h Outdated Show resolved Hide resolved

src/aof.c Outdated Show resolved Hide resolved

ranshid added 3 commits August 4, 2025 21:58

fix rdb version documentation

438564d

Signed-off-by: Ran Shidlansik <[email protected]>

remove unneeded warning log

51117ee

Signed-off-by: Ran Shidlansik <[email protected]>

fix format check error

d1e5c0a

Signed-off-by: Ran Shidlansik <[email protected]>

zuiderkwast approved these changes Aug 4, 2025

View reviewed changes

src/valkey-check-rdb.c Outdated Show resolved Hide resolved

src/valkey-check-rdb.c Outdated Show resolved Hide resolved

refactor a bit

97b0d9e

Signed-off-by: Ran Shidlansik <[email protected]>

sarthakaggarwal97 approved these changes Aug 4, 2025

View reviewed changes

ranshid added 2 commits August 4, 2025 22:25

validate rdb version 12-79 is not supported

329500f

Signed-off-by: Ran Shidlansik <[email protected]>

make valkey-rdb-check identical to server check

6a965ac

Signed-off-by: Ran Shidlansik <[email protected]>

madolson approved these changes Aug 4, 2025

View reviewed changes

zuiderkwast approved these changes Aug 4, 2025

View reviewed changes

madolson reviewed Aug 5, 2025

View reviewed changes

better verify prev versions megic

c20f1b7

Signed-off-by: Ran Shidlansik <[email protected]>

make ci use the latest 8.1.x for cross-version tests

a5cd03c

also make Cross version cluster - failover use relaxed rdb check Signed-off-by: Ran Shidlansik <[email protected]>

fix test Send cluster message via module to correctly track sent mess…

54a7d3f

…ages Signed-off-by: Ran Shidlansik <[email protected]>

zuiderkwast approved these changes Aug 5, 2025

View reviewed changes

zuiderkwast moved this to In Progress in Valkey 9.0 Aug 5, 2025

ranshid merged commit 33a43f2 into valkey-io:unstable Aug 5, 2025
52 checks passed

github-project-automation bot moved this from In Progress to Done in Valkey 9.0 Aug 5, 2025

hpatro reviewed Aug 5, 2025

View reviewed changes

ranshid mentioned this pull request Aug 6, 2025

Fix cluster test module to pass null terminated node-id to SendClusterMessage #2438

Closed

zuiderkwast mentioned this pull request Aug 11, 2025

Add compatibility test with Valkey 7.2/8.0 #2342

Merged

allenss-amazon pushed a commit to allenss-amazon/valkey-core that referenced this pull request Aug 19, 2025

bump the RDB version for Valkey 9.0(valkey-io#2422)

04a57ab

This is needed due to changes presented in valkey-io#2089 --------- Signed-off-by: Ran Shidlansik <[email protected]>

bump the RDB version #2422

bump the RDB version #2422

Uh oh!

Conversation

ranshid commented Aug 4, 2025

Uh oh!

codecov bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarthakaggarwal97 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranshid commented Aug 5, 2025

Uh oh!

ranshid commented Aug 5, 2025

Uh oh!

zuiderkwast commented Aug 5, 2025

Uh oh!

ranshid commented Aug 5, 2025

Uh oh!

ranshid commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ranshid commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Aug 4, 2025 •

edited

Loading

ranshid commented Aug 5, 2025 •

edited

Loading

zuiderkwast Aug 5, 2025 •

edited

Loading