Valkey release 8.1.0-rc1

[ranshid]

This is the PR for 8.1.0 release.
It includes:

1. Release note changes
2. version changes

[ranshid]

@valkey-io/core-team I hope I did fine with the release notes - please review and I will make the necessary changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.99%. Comparing base (c5ae37f) to head (49cbfab).
Report is 1 commits behind head on 8.1.

Additional details and impacted files

@@            Coverage Diff             @@
##              8.1    #1713      +/-   ##
==========================================
- Coverage   71.17%   70.99%   -0.19%     
==========================================
  Files         123      123              
  Lines       65536    65536              
==========================================
- Hits        46645    46525     -120     
- Misses      18891    19011     +120     

see 13 files with indirect coverage changes

[hwware]

I do not check the details, but I think we miss the Thanks part, please
reference this https://github.com/valkey-io/valkey/blob/7.2.8/00-RELEASENOTES

[zuiderkwast]

Good job!

Shall we make some noise about IFEQ? It might be a cool feature for some users.

[ranshid]

I do not check the details, but I think we miss the Thanks part, please reference this https://github.com/valkey-io/valkey/blob/7.2.8/00-RELEASENOTES

That was the most complicated work I ever did in Valkey. @hwware can you check the result looks good?

[madolson]

Cool. Mostly comment on lines which were most confusing to me.

[ranshid]

@valkey-io/core-team Thank you all for reviewing this!
I made attempt to address all of your comments.
Can you please approve once you are O.K with this content so that I know when is this good to me merged?

As a side note: For each contribution I took the PR title as the contribution statement. I think all your comments were good and valid, but I suggest for the future we should take extra care about reviewing and fixing the PR titles before merging them. specially when the PR is tagged with release-notes.

[ranshid]

So we hit some issue withRC versioning:
Currently, the way it was done in past releases, we used to follow the last major with PATCH number starting from 240.
This works for most cases but there are few caveats:

1. For external clients (eg valkey-cli) tests the support for new commands might break since new API are tagged with version support >= than the released version. for example: https://github.com/valkey-io/valkey/actions/runs/13281314141/job/37080127772?pr=1713
2. Some new implementations/tests rely on specific version checking (eg https://github.com/valkey-io/valkey/pull/1091/files)

There are several options we can choose:

1. keep the version formatting we decided on. This would also require us to fix the current tests as well as implement some version check logic in the code.
   For example having this code:

if (replica->repl_data->replica_version < 0x80100) 

might have to be changed to support version like 8.0.240 is treated the same as 8.1.0.
For example we could have functions providing version data and know how to treat the 240 and higher patch version logic, so it can later be used like:

if (valkeyGetMajorVersion(replica->repl_data->replica_version) < 8 || 
     valkeyGetMinorVersion(replica->repl_data->replica_version) < 1) 

2. giveup on the current minor version thing and still use 0,2,4,6,8 for GA versions and 1,3,5,7,9 for RCs

Other alternatives are welcome (We discussed some alternatives in the last maintainers meeting, but I was unable to recover all the options)

[zuiderkwast]

We were discussing some alternatives in a chat today. Some options are listed below. Whatever we decide, we can fix the failing tests that depend on version.

We can also consider the numeric VALKEY_VERSION_NUM. It's visible in the Lua API and module API.

• Use "8.0.240".
  • Pros: Most similar to old versioning scheme
  • Cons: Looks like a patch version of 8.0.
• Use "8.1.0".
  • Pros:
    • No broken tests.
    • COMMAND INFO "since" fields are correct for clients.
  • Cons:
    • No difference between release candidates.
    • May lead to compatibility issues between release candidates if we merge Trigger manual failover on SIGTERM / shutdown to cluster primary #1091 in RC2.
• Use "8.1.0-rc1" as version in INFO and HELLO and internally use numeric version 0x800f0 (8.0.240).
  • Pros:
    • The most correctly reported version.
    • RC visible in INFO and HELLO.
    • one-to-one conversion between numeric and string version.
    • Can prevent compatibility problems between release candidates.
  • Cons:
    • Can break some clients, but only in the release candidates.

[ranshid]

We were discussing some alternatives in a chat today. Some options are listed below. Whatever we decide, we can fix the failing tests that depend on version.

We can also consider the numeric VALKEY_VERSION_NUM. It's visible in the Lua API and module API.

• Use "8.0.240".

  • Pros: Most similar to old versioning scheme
  • Cons: Looks like a patch version of 8.0.

I think there are more cons here:

• It breaks some valkey-cli compatibility tests which we will have to fix and track for future testing
• code which aims to take version considerations will have to consider RC versions. For example
  Trigger manual failover on SIGTERM / shutdown to cluster primary #1091 is doing:

if (replica->repl_data->replica_version < 0x80100)

which will have to change to:

if (replica->repl_data->replica_version < 0x800f1)

which is strange given no one remembers RCs later on.

• Use "8.1.0".

  • Pros:

    • No broken tests.
    • COMMAND INFO "since" fields are correct for clients.

  • Cons:

    • No difference between release candidates.
    • May lead to compatibility issues between release candidates if we merge Trigger manual failover on SIGTERM / shutdown to cluster primary #1091 in RC2.

I actually like this option because it is the simplest one. I think we could improve the observability by introducing a "release-status" indication somewhere in the info section which will be rc1/2/3/...n for release candidates and 'ga' once we release the final candidate.

regarding the rc breakage - I am not sure how much effort we need to take for keeping RCs compatible? IMO we could offer no guarantees regarding RCs compatibility since they are released for a very limited time window.

• Use "8.1.0-rc1" as version in INFO and HELLO and internally use numeric version 0x800f0 (8.0.240).

  • Pros:

    • The most correctly reported version.
    • RC visible in INFO and HELLO.
    • one-to-one conversion between numeric and string version.
    • Can prevent compatibility problems between release candidates.

  • Cons:

    • Can break some clients, but only in the release candidates.

I think this option is good because of the clarity in the reporting. I think the cons are missing some maintenance effort and confusion that can happen when developing future "version aware" features and the need to understand which version to use (the VALKEY_VERSION or the VALKEY_VERSION_NUM). but I suppose these will not happen very often.

[madolson]

Use "8.1.0".

Would be my preference. The risk of breaking clients in the release candidates seems like the biggest thing that would prevent testing. We also talked about adding a separate field that "only" exists during the release candidate.

[madolson]

I feel more nit picky, but just generally trying to use ` consistently as well as trying to make the main paragraph a little bit more relevant and succinct.

[PingXie]

LGTM. Great work! Thanks @ranshid

[madolson]

Ok, I'm happy with it now. The only thing I'm just now noticing is commandlog is missing from top level comment, but other than that I think it's pretty good.

[ranshid]

Ok, I'm happy with it now. The only thing I'm just now noticing is commandlog is missing from top level comment, but other than that I think it's pretty good.

what do you mean by commandlog? you mean the slow commands log?

[zuiderkwast]

what do you mean by commandlog? you mean the slow commands log?

Yes, slow and fat. Large request and large response log.

It's an admin feature. Maybe not that exciting for users? They only care when they have a problem... WDYT?

[madolson]

Yes, slow and fat. Large request and large response log.

Yeah, there is a new command called commandlog for observability. Which has been a pretty commonly requested features.

[ranshid]

Yes, slow and fat. Large request and large response log.

Yeah, there is a new command called commandlog for observability. Which has been a pretty commonly requested features.

I know the new feature, just did not understand it's relation to the top-comment. you mean the top comment in the notes. I understand now. I can add something. thank you!

[soloestoy]

LGTM, and agree it's better to mention commandlog in top level