Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

redis_version reports "6.2.11" #4256

Open
mperham opened this issue Dec 4, 2024 · 5 comments · May be fixed by #4286
Open

redis_version reports "6.2.11" #4256

mperham opened this issue Dec 4, 2024 · 5 comments · May be fixed by #4286
Assignees
@adiholden

Comments

@mperham
Copy link

mperham commented Dec 4, 2024

Hi, I'm the author of Sidekiq which supports Redis, Valkey and Dragonfly as its backing store. I'm working through a major new 8.0 release coming in 2025Q1, which includes removing support for older, unsupported Redis versions.

Redis 7.0 and 7.1 are unsupported, 6.2 will be unsupported very soon. To that end, I'd like to require Redis 7.2 going forward. I envision supporting 7.2 for years to come due to licensing issues.

This requirement works fine with Redis and Valkey but Dragonfly latest is still reporting 6.2.11 as its "redis_version" in the INFO output. Any tips/advice on how to proceed? I could check the actual Dragonfly version but I need to validate that your engine has Redis 7.2 features so I can use them. Am I limited to 6.2 features if I want to support Dragonfly?

https://github.com/sidekiq/sidekiq/blob/main/lib/sidekiq/cli.rb#L77
@adiholden
Copy link
Collaborator

Hi @mperham
Thanks for reaching out!
You are not limited to redis 6.2 when using dragonfly, but I believe we have some gaps with redis 7.2 api.
Did you try runinng dragonfly with the new release and see errors reported by dragonfly that we are not supporting some apis that you use? If not suggest doing so, I guess this will be the easiest way to know if we are missing something that you need.
Or you can list here your changes in your new version regarding redis apis and we can respond if there is something we believe is missing.
Thanks:)

@mperham
Copy link
Author

mperham commented Dec 4, 2024

I don't have any functional issues as of yet since I haven't started the process of reviewing and refactoring to use those >6.2 changes. This was my first step to do so.

@adiholden
Copy link
Collaborator

I see. In this case it will be hard to tell if we miss some functionality that you will need. I suggest to check against latest dragonfly version and as you progress with new changes if we miss something you can open an issue for it, and once the fix is in you can check against the dragonfly version that has this new feature.

@mperham
Copy link
Author

mperham commented Dec 5, 2024

Well, my CI runs all three engines against the test suite so we've got pretty good coverage there, that's how I found out about this issue in the first place. The problem is that I don't want to have to add special logic for each engine version check. I'd rather just check against your reported redis_version. I'm happy to drop back down to 7.0 if that helps, it doesn't look like 7.{1,2} added much relevant functionality for my needs but I could use the NX flag on EXPIRE in 7.0.

@adiholden
Copy link
Collaborator

Looking into our code commit history it looks like we started returning redis_version to support Sidekiq client :)
We will make the change to report redis_version 7.2

@adiholden adiholden self-assigned this Dec 10, 2024
@adiholden adiholden linked a pull request Dec 10, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adiholden adiholden

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

server: report redis version 7.2.0 to support Sidekiq dragonflydb/dragonfly
2 participants
@mperham @adiholden