JedisPool and Pipeline

[steven b]

I'm new to Redis and was experimenting with Pipeline and JedisPool and
one of the interactions between them seem less than ideal: if you fail
to run sync() on the pipeline, the connection returned to the pool
will be out of sync and future re-use of the object can cause, in the
context of the re-using code, very strange results. I would imagine
there are similar issues with respect to transactions, but my use
cases don't involve them.

Since you're supposed to call Pipeline.sync() on pipelines I'm not
sure if this is an actual bug, but the thought of having to use
"finally" on every pipeline usage seemed like a poor idea, so I
overrode BaseObjectPoolFactory.passivateObject as follows:

public void passivateObject(final Object obj) throws Exception
{
if (obj instanceof Jedis) {
final Jedis jedis = (Jedis) obj;
final Client client = jedis.getClient();

if (client.isInMulti())
client.discard();
client.getAll();
}
}

client.getAll() seems a bit wasteful here since we aren't doing
anything with the results, but I couldn't see anything else that
clears out pipelined commands. I'm attempting to discard transactions
because that behavior is consistent with what our database connection
pooling does. It seems to work fine, but I just wanted to make sure I
wasn't missing something.

[SBoyle]

As far as I know that is just the way redis works, not really a jedis
thing. Redis wants to send the response not dump the response.
http://redis.io/topics/pipelining

[Jonathan Leibiusky]

So you are saying that when you get an error in the middle of the pipeline and you return the jedis instance to the pool you run into strange errors?
First of all, why you would get an error in the middle of the pipeline? Is it an application exception or something to do with Jedis?
Second, when you get a JedisConnectionException you should ALWAYS discard the jedis instance by calling pool.returnBrokenResource(jedisInstance). This way you won't have problems and you won't need to override passivate and call getAll()

[steven b]

I ran across this because I got a non-Jedis related exception between
using a Pipeline and calling Pipeline.sync(), meaning Pipeline.sync()
was never called.

Note that the "strange errors" was not a Jedis exception. In my
particular scenario, I was getting a ClassCastException:
java.lang.ClassCastException: java.lang.Long cannot be cast to
java.util.List
at
redis.clients.jedis.Connection.getBinaryMultiBulkReply(Connection.java:
189)
at
redis.clients.jedis.Connection.getMultiBulkReply(Connection.java:182)
at redis.clients.jedis.Jedis.smembers(Jedis.java:1176)

Here's some sample code that demonstrates how I was getting a
ClassCastException:

Jedis jedis = pool.getResource();
try {
Pipeline p = jedis.pipelined();
p.sadd("some_key", userId);
p.setnx("some_key_" + userId,
Long.toString(System.currentTimeMillis()));
p.expire("some_key_" + userId, 60);

// some exception is thrown
if (true)
throw new Exception("hi there");

p.sync();
} finally {
pool.returnResource(jedis);
}

// Then, later, if we get the Jedis object the above code used:
Jedis jedis = pool.getResource();
try {
jedis.smembers("some_key"); // this will throw a
ClassCastException
} finally {
pool.returnResource(jedis);
}

From what I could tell the ClassCastException was because my first
pipeline call was to sadd, which has an integer reply, and when I
later tried to re-use the client (after returning it to the pool then
getting it back from the pool) I called smembers, which uses
getMultiBulkReply. It seems like depending upon which calls to the
Jedis object were made you may get incorrect return values instead of
an exception.

Note that I was using the default configurations on the JedisPool -
validateObject returns false when the above happens, so if you have
"test on borrow" or "test on return" set to true destroyObject will be
called.

[Jonathan Leibiusky]

Ok. This is great!
The problem is that you are opening a pipeline and returning jedis to the pool without closing it (this happens when you throw the exception), meaning that the pool now has a broken jedis instance that could be retrieved on a next getResource() and since you want to do something but there is a pipeline in the middle of something already, all the different expected values are messed up.

The way to handle this is to call returnBrokenResource instead of returnResource.
That tells the pool to kill that instance and it is guaranteed that you won't even get it on getResource calls.

Please try that and let me know!!! And thanks for taking the time to reproduce this!!!

Jonathan