Skip to content

Conversation

@satheeshaGowda
Copy link
Contributor

@satheeshaGowda satheeshaGowda commented Aug 26, 2025

===========NOT TO BE MERGED======

This patch extends Redis 6.0 to support roll forward upgrade and roll backward downgrade compatibility with

  • RDB v9: Redis 6.2
  • RDB v10: Redis 7.0
  • RDB v11: Redis 7.2
  • RDB v11: Valkey 7.2
  • RDB v11: Valkey 8.0

Listpack to Ziplist Conversion Process

  1. Parse listpack header and validate structure
  2. Walk through each listpack element
  3. Extract element data (string or integer)
  4. Add each element to new ziplist structure
  5. Return converted ziplist for normal processing

…key 7.2/8.0

Signed-off-by: Satheesha Chattenahalli Hanume Gowda <[email protected]>
@satheeshaGowda satheeshaGowda changed the title Redis/RDB backward downgrade compatibility from Redis 7.0/7.2 and Valey 7.2/8.0 Redis/RDB backward downgrade compatibility from Redis 7.0/7.2 and Valkey 7.2/8.0 Aug 26, 2025
@satheeshaGowda satheeshaGowda changed the title Redis/RDB backward downgrade compatibility from Redis 7.0/7.2 and Valkey 7.2/8.0 Redis/RDB backward downgrade compatibility from Redis 7.0/7.2 and Valkey 7.2/8.0 to Redis 6.0 Aug 26, 2025
@satheeshaGowda satheeshaGowda changed the title Redis/RDB backward downgrade compatibility from Redis 7.0/7.2 and Valkey 7.2/8.0 to Redis 6.0 Roll backward downgrade compatibility from Redis 7.0/7.2 and Valkey 7.2/8.0 to Redis 6.0 Aug 26, 2025
Copy link
Collaborator

@hpatro hpatro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting the change along with #2550 #2551.

Just a high level comment:

Could we make these change as individual commits to be able to stack on top of each other. Will be easier for us to create for 8.1/9.0 in the future.

OR

Individual commits based on the feature support like listpack to ziplist, hostname support, etc. This would enable folks to easily pick it for their needs and support older version if required in their case.


/* Node coordinates */
ci = sdscatprintf(sdsempty(),"%.40s %s:%d@%d ",
ci = sdscatprintf(sdsempty(),"%.40s %s:%d@%d",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need to add hostname support, we would need to bring in extensions mechanism to older version as well.

node->ip,
node->port,
node->cport);
if (node->hostname && sdslen(node->hostname) > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add a nodes.conf file with hostname in it and verify it gets loaded fine.

@zuiderkwast
Copy link
Contributor

See comment #2551 (comment)

@hpatro
Copy link
Collaborator

hpatro commented Aug 27, 2025

Continuing the conversation here as it's the largest of all changes.

@zuiderkwast IIUC the intention here is not to merge into different branch of this project (already signalled by @satheeshaGowda by "NOT TO BE MERGED"). Rather with this PR we are able to visaulize the diff and provide feedback. We could maintain these patches independently under the Valkey organisation in a separate repo and folks can use it for their usecases where they might need to rollback after usage of new features in the latest version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants