search

valkey-io / valkey

A flexible distributed key-value datastore that supports both caching and beyond caching workloads.
https://valkey.io
Other
16.38k stars 611 forks source link

Integrate the bugfixes of Redis 7.2.5 #561

Open mkauf opened 4 months ago

mkauf commented 4 months ago

Redis 7.2.5 has been published: https://github.com/redis/redis/releases/tag/7.2.5

This release uses the "old" Redis license.

Please integrate the bugfixes of this release into Valkey.

SoulPancake commented 4 months ago

@mkauf @madolson @zuiderkwast Folks, Can I take this up

madolson commented 4 months ago

@SoulPancake Yeah. If you can, submit a PR cherry-picking the commits from unstable into the 7.2 branch. Thanks! I did review the list of changes from 7.2.5, and it looked like they were all commits from before the license change.

hwware commented 4 months ago

@SoulPancake Yeah. If you can, submit a PR cherry-picking the commits from unstable into the 7.2 branch. Thanks! I did review the list of changes from 7.2.5, and it looked like they were all commits from before the license change.

License changes from redis 7.4, so now cherry-picking is safe

madolson commented 4 months ago

Btw, we should have all the commits in our development tree, so you should ideally find the relevant commits from our unstable branch and cherry-pick those.

enjoy-binbin commented 4 months ago

I think for safety reasons (or for better conflict resolution), the release branch should be arranged by core-team members since in think we won't review it very carefully sometimes.

zuiderkwast commented 4 months ago

@enjoy-binbin This makes sense.

But to help prepare this, we first need to find the commits that we need to cherry pick. They can be posted as comments in this issue. That could save some time.

SoulPancake commented 4 months ago

These were the commits b3aaa0a1362d229ba1ecd44629655f76c77304ec 1f00c951c27f0332fd8e703beaf9e28cb404a0b4 492021db95288be2095ec7f1db2ee043fac3c2d9 5fdaa53d20c82c99042e79b737fb9bc157a73d60 763827c98144fa83b4fc50eb4cae7ec4d63e7925 da727ad445a640950baa097124070468a0316cc9

However, I guess there were some refactoring in valkey unstable on top this that also needed to be included

SoulPancake commented 4 months ago

think for safety reasons (or for better conflict resolution), the release branch should be arranged by core-team members since in think we won't review it very carefully sometimes.

I agree with this, It's trickier than I thought to attempt the conflict resolution or choose which further commits to include I am closing https://github.com/valkey-io/valkey/pull/567

hwware commented 4 months ago

Today I check the commit in Reds 7.2.5, I think all code changes already are included in our unstable branch, please other core team member to double check Thanks @valkey-io/core-team

hwware commented 3 months ago

To close it because we have these parts code already

zuiderkwast commented 3 months ago

The plan is to cherry-pick the commits to our 7.2 branch and make a patch release.

madolson commented 3 months ago

Btw. I still think this overall approach makes sense. In Redis, one of the maintainers did all the backporting, but ideally we should do the backport with a PR.

murphyjacob4 commented 3 months ago

@zuiderkwast @madolson I can send the backport PR

LiiNen commented 3 months ago

some of commits referred in https://github.com/valkey-io/valkey/pull/624 are already included. (as i know) fork of valkey is created since redis' license changes. so only we need is to cherrypick commits after changing will be fine.

LiiNen commented 3 months ago

btw, just wondering.. I have not yet checked whether exists or not, but, if some commits approved in redis 7.4 and this commit has been applied also in 7.2 as backport, then can we cherrypick it?

hwware commented 3 months ago

I already checked our Valkey commit history, the 6 commits in Redis 7.2.5 already are included, this is why I close this issue. Below is the detail PRs:

A single shard cluster leaves failed replicas in CLUSTER SLOTS instead of removing them (#12824) https://github.com/valkey-io/valkey/commit/b3aaa0a1362d229ba1ecd44629655f76c77304ec Crash in LSET command when replacing small items and exceeding 4GB (#12955) https://github.com/valkey-io/valkey/commit/1f00c951c27f0332fd8e703beaf9e28cb404a0b4 Blocking commands timeout is reset due to re-processing command (#13004) https://github.com/valkey-io/valkey/commit/492021db95288be2095ec7f1db2ee043fac3c2d9 Conversion of numbers in Lua args to redis args can fail. Bug introduced in 7.2.0 (#13115) https://github.com/valkey-io/valkey/commit/5fdaa53d20c82c99042e79b737fb9bc157a73d60 redis-cli: --count (for --scan, --bigkeys, etc) was ignored unless --pattern was also used (#13092) https://github.com/valkey-io/valkey/commit/763827c98144fa83b4fc50eb4cae7ec4d63e7925 redis-check-aof: incorrectly considering data in manifest format as MP-AOF (#12958) https://github.com/valkey-io/valkey/commit/da727ad445a640950baa097124070468a0316cc9

Because all these PRs are in the Redis 7.2.5, they are safe in our Valkey repo.