Make JedisFactory as public class

Beacause we need to write the JedisPool/SentinelPool ourself in the project the match the project requirements, we hope to reuse the JedisFactory, but it is not a public class,

[xetorthio]

Can you provide more details on what you need to do?
On Feb 8, 2014 1:36 AM, "Calvin Xiao" notifications@github.com wrote:

Beacause we need to write the JedisPool/SentinelPool ourself in the
project the match the project requirements, we hope to reuse the
JedisFactory, but it is not a public class,

Reply to this email directly or view it on GitHubhttps://github.com//issues/533
.

we will write a MyJedisPool class, which is similar to JedisPool, so we wish to use JedisFactory,

[HeartSaVioR]

Will you modify many things from JedisPool?

If so, please describe what points of JedisPool are needed to override and improve, for future of Jedis.
If not, there's an workaround to use JedisFactory outside of Jedis by making public class MyJedisFactory and set package to redis.clients.jedis.
(Actually it's not beautiful - somewhat dirty, but I've done with twitter4j already. I have no choice to do it.)

Hope this help :)

I do below modification:

• Introduce a ConnectionInfo class to wrap the connection information.
• Modify the Sentinel Pool , try to make it more robutness.

Because I don't think xetorthio will need these, so i didn't send the pull request :)

you can check my code here:
https://github.com/springside/springside4/tree/master/modules/extension/src/main/java/org/springside/modules/nosql/redis/pool

[marcosnils]

@springside I don't totally understand the necessity for the ConnectionInfo class. You can pretty much get everything you need from the BinaryClient class in Jedis.

When you say that you made Sentinel Pool more robust, what changes did you make to it?. Maybe there's something that we can actually implement in Jedis.

@marcosnils @HeartSaVioR @xetorthio
Thank you for your quickly reply.

ConnectionInfo is not nessary for Jedis, our team like to use it to reduce the parameters and member fields in JedisPool and SentinelPool, but i don't think Jedis need this modification which would break previous code.

Current Sentinel Pool assumes the Sentinel Cluster would work well (at least one instance will live), so it only query the master at the beginning and just listen the master switch....but if all sentinel and some redis instance restart in the long lifecycle of the application, the pool can't workwell, so i change the flow to handle these cases.

On the other hand, current Sentinel Pool only catch the JedisConnectionException, i think we also need to catch RuntimeException to avoid the unexpected runtimeexception break the thread.

[HeartSaVioR]

@springside Hello!
I agree that Sentinel Pool can be more robust.

I think JedisSentinelPool is currently lacking of dealing with some statuses.
(NOT a bug) 1.-beginning (first master recognition is not ended) : It could throw NullPointerException from Pool's method using internalPool
<= it doesn't return until initializing sentinel and pool.
2. at moment between master is recognized to odown and switch-master : JedisSentinelPool can't determine master is down, and also Sentinel aborts failover. JedisSentinelPool only waits "+switch-master"... It doesn't care about failover process.
3. MasterListener Thread is broken due to other Exception (not JedisConnectionException) : as you mentioned.

Also I think JedisSentinelPool's behavior if sentinel cluster goes down is a policy.

1. Master can be alive during all Sentinel are down

   (current implementation) JedisSentinelPool is no problem at using current master. So let it go. :)

2. We can't believe current master is real master. What about sentinel cluster changes master during JedisSentinelPool sleep 5000ms for reconnect all sentinel?

   We need additional status for "all sentinel are down".
   And after reconnecting, JedisSentinelPool should ping to Sentinel if master has changed.
   (we can't receive message from that moment)
   <= JedisSentinel should find Sentinel with latest master information. Applying any Sentinel's master information is a bit dangerous with network partitioned. Currently JedisSentinelPool's implementation has same issue.

Actually I like latter approach. :) (with strengthen description)
I wish to make some time for JedisSentinelPool more robust.
And I suggest you to help us to let JedisSentinelPool more robust too. :)

Glad to see that my issue trigger that JedisSentinelPool to be more robust, looking forward it:)