You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "sekikn (via GitHub)" <gi...@apache.org> on 2023/11/08 09:50:45 UTC
[PR] Bump Jedis version to 5.0.2 (druid)
sekikn opened a new pull request, #15344:
URL: https://github.com/apache/druid/pull/15344
### Description
Currently, the redis-cache extension uses Jedis 2.9.0, which was released over seven years ago and [is no longer listed in the official support matrix](https://github.com/redis/jedis#supported-redis-versions). This patch upgrades it to ensure the compatibility with the recent version of Redis and make future upgrades easier, including:
* Upgrade Jedis to v5.0.2, the latest version at this writing, and address the API changes and dependency version mismatch.
* Replace mock-jedis with jedis-mock, since the former has not been actively maintained any longer and not compatible with recent versions of Jedis.
This PR has:
- [x] been self-reviewed.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Bump Jedis version to 5.0.2 (druid)
Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on code in PR #15344:
URL: https://github.com/apache/druid/pull/15344#discussion_r1386596750
##########
extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java:
##########
@@ -110,10 +109,6 @@ protected Pair<Integer, Map<NamedKey, byte[]>> mgetFromRedis(Iterable<NamedKey>
@Override
protected void cleanup()
{
- try {
- cluster.close();
- }
- catch (IOException ignored) {
- }
+ cluster.close();
Review Comment:
why this change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Bump Jedis version to 5.0.2 (druid)
Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 commented on PR #15344:
URL: https://github.com/apache/druid/pull/15344#issuecomment-1801864876
LGTM. Thank you for your contribution @sekikn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Bump Jedis version to 5.0.2 (druid)
Posted by "sekikn (via GitHub)" <gi...@apache.org>.
sekikn commented on code in PR #15344:
URL: https://github.com/apache/druid/pull/15344#discussion_r1386684776
##########
extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java:
##########
@@ -110,10 +109,6 @@ protected Pair<Integer, Map<NamedKey, byte[]>> mgetFromRedis(Iterable<NamedKey>
@Override
protected void cleanup()
{
- try {
- cluster.close();
- }
- catch (IOException ignored) {
- }
+ cluster.close();
Review Comment:
Thank you for the review @abhishekagarwal87! In Jedis 5.0.2, that method is inherited from the `UnifiedJedis` class and [it doesn't raise IOException since `IOUtils.closeQuietly` is internally used](https://github.com/redis/jedis/blob/v5.0.2/src/main/java/redis/clients/jedis/UnifiedJedis.java#L233-L236).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Bump Jedis version to 5.0.2 (druid)
Posted by "abhishekagarwal87 (via GitHub)" <gi...@apache.org>.
abhishekagarwal87 merged PR #15344:
URL: https://github.com/apache/druid/pull/15344
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org