Cluster pipelining

[sdncha]

Added pipelining support to JedisCluster, making least changes on other classes. The idea is capturing all commands sent with pipeline, and replaying them for cluster redirecting.

[marcosnils]

@sdncha thx for the contribution!. This PR looks promising and hopefully we can merge soon. It'll probably take me some time but I definitely would like to include it. @HeartSaVioR if you have some time I'd like your help to review.

[sdncha]

@marcosnils , glad to hear that. I look forward to your reviews and would like to improve it, make it good enough to be merged.

[HeartSaVioR]

@sdncha
Thanks for the contribution. Could you refer the latest comments on #1527 to see my theory on this side, and put your theory?
Please note that only idempotent operations could be placed to replay.

[HeartSaVioR]

You may want to revisit the discussion prior to put your theory: https://groups.google.com/d/msg/redis-db/4I0ELYnf3bk/Lrctk0ULm6AJ

[ChetanBhasin]

I was wondering what the status of this PR is. Is anyone still looking at it?

[rkass]

Any updates on this?

[ucjonathan]

+1 for support for pipelining on a cluster. Would like to continue using Jedis libraries to talk to Elasticache without having to rewrite a bunch of existing code since pipelining is not supported. Pipelining is apparently is supported by the lettuce.io library, but rewriting everything is even less appealing.

[mpermar]

This would be a real nice feature.

[saberem]

@marcosnils @sdncha @HeartSaVioR Any further discussion on this given the latest comments on #1527?

[Unknow0]

Hi @sdncha,

when we had a network issue we had a lot of unclosed connection plus we had a lot of these exception:

redis.clients.jedis.exceptions.JedisException: Could not return the resource to the pool
    at redis.clients.jedis.JedisPool.returnResource(JedisPool.java:247)
    at redis.clients.jedis.Jedis.close(Jedis.java:3449)
    at redis.clients.jedis.JedisClusterPipeline.releaseConnection(JedisClusterPipeline.java:278)
    at redis.clients.jedis.JedisClusterPipeline.reset(JedisClusterPipeline.java:288)
    at redis.clients.jedis.JedisClusterPipeline.sync(JedisClusterPipeline.java:314)
    at redis.clients.jedis.JedisClusterPipeline.syncAndReturnAll(JedisClusterPipeline.java:330)
// [..]
Caused by: redis.clients.jedis.exceptions.JedisException: Could not return the resource to the pool
    at redis.clients.util.Pool.returnResourceObject(Pool.java:64)
    at redis.clients.jedis.JedisPool.returnResource(JedisPool.java:244)
    ... 12 more
Caused by: java.lang.IllegalStateException: Object has already been returned to this pool or is invalid
    at org.apache.commons.pool2.impl.GenericObjectPool.returnObject(GenericObjectPool.java:551)
    at redis.clients.util.Pool.returnResourceObject(Pool.java:62)
    ... 13 more

i didn't at time to look more into it :s

Regards.

[lcy362]

Any update?

[ucjonathan]

I'd also like to see cluster pipelining as well. Tons of older code that we would have to re-write just to move to newer cluster deployment of redis. Can we move past the religious debate about whether there should be pipelining and add support for it? Have it off by default if you like.

[funaiy]

Any update?

[jaronoff97]

Any updates? It's been more than three years now 😅

[ucjonathan]

I just bit the bullet and refactored the code to use JedisCluster and stop pipelining. It actually wasn't as bad as I had expected it to be.

[marcosnils]

I just bit the bullet and refactored the code to use JedisCluster and stop pipelining. It actually wasn't as bad as I had expected it to be.

:D

[esigma5]

Any update? Argentina won their third football world cup but this PR is still not merged 🙏

[sazzad16]

@esigma5 Give it at least 36 years.

Enjoy ClusterPipeline for now.

jedis/src/main/java/redis/clients/jedis/ClusterPipeline.java

Line 8 in 022a9af

public class ClusterPipeline extends MultiNodePipelineBase {