Skip to content

Conversation

michaldo
Copy link
Contributor

@michaldo michaldo commented Oct 3, 2025

Drop support for Redis 3, which is not maintained since 2018

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

After discussion with Spring Data team, we agreed that fully dropping DEL support is fair game from now on.
Please, consider to refine the logic just deal with UNLINK.
Also, take a look into RedisQueueInboundGateway, RedisQueueMessageDrivenEndpoint and RedisMessageStore where we still use DEL.

Thanks

@michaldo michaldo changed the title Carefully switch from Redis unlink to delete Drop switch from Redis unlink to delete Oct 3, 2025
@michaldo
Copy link
Contributor Author

michaldo commented Oct 3, 2025

I decided to remove switch from unlink to dev
About places where del is used, I will take a look later

@michaldo michaldo requested a review from artembilan October 3, 2025 15:38
@artembilan
Copy link
Member

I will take a look later

Thank you!

Let's us know, please, if you are going to that as a part of this PR or so!
In the end we would need to mention in the whats-new.adoc such a change in Redis module.
Also, the redis.adoc need to be revised if we still mention DEL somewhere there and probably an algorithm you've just dropped.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Also, just noticed:

Signed-off-by: michaldo <michaldo@github.com>

Please, use your legal name instead according to DCO requirements.
You can configure your Git client to produce a proper name for you into those commits.

@michaldo
Copy link
Contributor Author

michaldo commented Oct 3, 2025

This PR is functionally closed. I encountered problem of not careful switch, analyzed it and fixed. I am sure this change is safe because no modern application should be switched to delete mode.
Regarding other places where delete is used, I'm not familiar with context. I'm going to take a look.

Michal Domagala added 4 commits October 3, 2025 18:34
More performant `unlink` is not available on Redis 3.

Implementation is aware that, however not careful exception handling may switch to less performant `delete` on connection exception or similar case. The switch is permanent.

Maybe better choice is drop support for Redis 3, which is not maintained since 2018

Signed-off-by: Michal Domagala <michaldo@github.com>
Redis 3, target of fallback, is not maintained since 2018

Signed-off-by: Michal Domagala <michaldo@github.com>
Remove code remainings

Signed-off-by: Michal Domagala <michaldo@github.com>
Signed-off-by: Michal Domagala <michaldo@github.com>
Signed-off-by: Michal Domagala <michaldo@github.com>
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Please, add your name into the @author list of the affected class.
Also, give us your thought about other classes in Redis module which still use DEL.

Thank you again!

Michal Domagala added 2 commits October 3, 2025 19:47
Signed-off-by: Michal Domagala <michaldo@github.com>
Signed-off-by: Michal Domagala <michaldo@github.com>
@michaldo
Copy link
Contributor Author

michaldo commented Oct 3, 2025

I add myself as one of @author's
Regarding other classes:

  • RedisMessageStore: is it same case switch from unlink -> delete and I removed it also

  • RedisQueueInboundGateway, RedisQueueMessageDrivenEndpoint: they use delete command within JMX @ManagedOperation command. I feel that JMX action looks like admin action and I think If were someone who executes admin action I prefer to be sure that when action is completed everything is done. It mean I prefer synchronous delete over asynchronous unlink

@artembilan
Copy link
Member

I prefer to be sure that when action is completed everything is done

Sounds reasonable.
That was oversight on my side when I performed search for RedisTemplate.delete() 😄

Thank you for looking over there!

@artembilan artembilan merged commit 1e71d19 into spring-projects:main Oct 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants