ArrayIndexOutOfBoundsException in RedisOutputStream using JedisPool in 1.5 about jedis HOT 21 CLOSED

I've created a test class to mimic the production behavior of hammering hget / hincrBy concurrently with many threads that access Jedis through a shared JedisPool, but I have yet to be able to repro this. Will update if I do.

from jedis.

I should also note that this is using redis 2.0.4

from jedis.

which version of jedis are you using?

from jedis.

That would be 1.5 final. Whatever happened, it's quite obscure, because I had a machine running that concurrency test I mentioned for a couple hours without a single failure. So, I made one change to use returnBrokenResource when returning a jedis to the pool if an exception had been thrown, and then deployed in production again. This time we're good so far, going on about an hour now.

from jedis.

so it might be that the pool is not handling broken connection.
in the apache pool you have several ways to address this.
you can set the text on borrow to true or test on return. you can also tell the pool to test idle connection.

from jedis.

I thought I'd note that I am also experiencing this bug using 1.5.0-RC2.

This is with a long running client doing a lot of smembers ,del and sadd,

from jedis.

did you try the above recommendations? setting the test on borrow to true?

from jedis.

I haven't tried the test on borrow because I have not had this issue since I started using returnBrokenResource when an exception was thrown while using the resource. It seems logical that test on borrow would also alleviate the problem, but at the cost of the check itself every time you need a resource.

from jedis.

well... there is no perfect solution.
redis by default closes connections every 300 seconds. The only way to be 100% sure that you don't get from the pool a broken connection is either test on borrow or tell redis not to close connections never by changing the configuration file.

from jedis.

Good to know. I had seen occasional errors saying it appeared the connection had been closed, but it hadn't been causing any problems since the code was set to auto retry on error (with a limit, clearly). In your experience, has the conf change to not close connections caused problems?

from jedis.

if you trust your clients and you know the amount of maximum connection you'll have I would say you can just tell redis not to close the connections at all.
although if redis shutdown forcibly you wouldn't catch those errors in the pool.
there are lots of options in the apache pool. you can actually tell it to test idle connections so you won't spend time doing it on every getResource() call.
So my recommendation is probably to tell redis not close connections and tell the pool to test idle connections, as long as you trust your clients and know how many connections you'll have maximum.

from jedis.

I switched from using one redis client from the pool eternally to returning and retrieving a new Jedis instance every so often (ca twice a sec) and also setting testBorrow = true . It now hums along nicely.

from jedis.

I also updated the Wiki with a bit info.

from jedis.

Perfect! Thanks a lot for updating the wiki. I will close this issue for now.

from jedis.

I don't think you should close this issue. Things should work smoothly by simply taking connections and returning them within the 300s. The client shouldn't be responsible for the health of the pool connections IMHO.

from jedis.

I don't agree. A connection pool is something that should be carefully configured for every application, because every application will have different needs.
That's why we are using Apache Common Pool, which has lots of different configurations.
JedisPool receives a Config object, which is responsible for how the pool will behave. Every user should decide if to use default values or change them according to specific needs.
All the configurations are here: http://commons.apache.org/pool/apidocs/org/apache/commons/pool/impl/GenericObjectPool.html
So in my opinion it is the user's task to decide if defaults are OK or not.

from jedis.

I agree with @Idsimonassi, being able to customize the pool for unique application needs is important. The main problem here was that this behavior was undocumented and so each developer discovered it under frustrating circumstances. Now that it's mentioned in the wiki, though, I think the issue is resolved.

from jedis.

I like the configurability and am glad you're using a well known pool implementation. I just don't think the default behavior should be to eventually fail when using it with the standard getResource() / returnResource() calls. All of the configuration on the pool should be for handling actual exceptional conditions. For example, if the default behavior is that Redis will close connections idle longer than 300s then the pool should by default be configured to work under those conditions without strange failures like this one.

As much as you would like every developer to configure the pool carefully they really don't know as much about Redis or Jedis as the developer and they should get as much help as possible. Checking the Wiki, I don't really see any best practices outlined there that would help a developer do something like:

setMinEvictableIdleTimeMillis( < 300s)
startEvictor( <150s)

That might alleviate this issue. For example, I just went and set testWhileIdle but now I realize that isn't enough as you probably also need timeBetweenEvictionRunsMillis set < 300s so that it actually gets tested. Since you have a constructor for the pool that doesn't take a Config object, I would use that opportunity to setup a very conservative configuration that is unlikely to fail under normal circumstances.

from jedis.

If anyone can comment, I ended up with this configuration to try and ensure that this doesn't happen again:

GenericObjectPool.Config poolConfig = new GenericObjectPool.Config();
poolConfig.testWhileIdle = true;
poolConfig.minEvictableIdleTimeMillis = 60000;
poolConfig.timeBetweenEvictionRunsMillis = 30000;
poolConfig.numTestsPerEvictionRun = -1;
JedisPool pool = new JedisPool(poolConfig, "localhost", 6379);

Thanks,
Sam

from jedis.

This is interesting. Probably we can change the default configuration of the pool so it will alleviate most of the problems related with this.
I strongly believe that "defaults matters".
Lets do it! we can document it properly and the user can always change the configuration for his own needs.
Although this won't be part if this issue but a new and fresh one :)
Please create a new issue with the proposed default configuration, we will wait for input from other developers and eventually set that to the default.

from jedis.

+1
Just one minor nitpick: Remember to add documentation to the JavaDocs as well as the Wiki.

from jedis.