Baqend/Orestes-Bloomfilter

jedis pool

So-Far-So-Good opened this issue · 7 comments

you can use JedisSentinelPool instead of JedisPool, What do you think?

Do you guys have some direction on changing the RedisPool to use a Pool and provide a way to define which type of Jedis pool to use? I'm thinking about taking this on since we need Sentinel support in our application and wanted to make sure you guys haven't already done it/thought more about it.

Thanks

Hi is this project still alive?
I tested with sentinel and was able to run as expected with 3 nodes, when forcing the original master to crash the library connected to the new master no problems.
Before I refactor all of the tests to support knowing if running in Sentinel, can you give me some direction about my ideas in the WIP pull request?

Not sure, whether my mail reply came through, therefore pasted here again:

Hi Chris,

sorry for the long delay. This project is absolutely still alive, it's a central part of our SaaS and we've just raised a substantial funding round.

We haven't yet started with support for Sentinel and Redis Cluster but will do that in the near future.

should I build a RedisPoolConfiguration class similar to the RedisSentinelConfiguration for consistency?

Yes, I think that is a good idea.

I see a pull request for SSL support. if I add RedisSentinelConfiguration I think it would make sense to add all new pool configuration settings to it, instead of expanding the set of Constructors.

Definitely!

what does the startThread logic do? I couldn't find any tests or where it is used. If needed, can you explain more about why you're creating an explicit Jedis instead of using a resource from the pool?

It is not yet used in the normal Bloom filters. Its purpose is to handle failure-tolerant pubsub connections. You can leave it unchanged.

I've looked at Redis Cluster in Jedis enough to see I can't do the Pool change and have it work with Cluster too. Are you guys working on a refactoring that would work with any connection source?

We haven't started that but we want to add Redis Cluster support soon.

Happy to answer any other questions. Let me know when I should merge your pull requests.

Best,
Felix

Hi Chris,

sorry for the long delay. This project is absolutely still alive, it's a
central part of our SaaS and we've just raised a substantial funding round.

We haven't yet started with support for Sentinel and Redis Cluster but will
do that in the near future.

should I build a RedisPoolConfiguration class similar to the
RedisSentinelConfiguration for consistency?

Yes, I think that is a good idea.

I see a pull request for SSL support. if I add RedisSentinelConfiguration
I think it would make sense to add all new pool configuration settings to
it, instead of expanding the set of Constructors.

Definitely!

what does the startThread logic do? I couldn't find any tests or where it
is used. If needed, can you explain more about why you're creating an
explicit Jedis instead of using a resource from the pool?

It is not yet used in the normal Bloom filters. Its purpose is to handle
failure-tolerant pubsub connections. You can leave it unchanged.

I've looked at Redis Cluster in Jedis enough to see I can't do the Pool
change and have it work with Cluster too. Are you guys working on a
refactoring that would work with any connection source?

We haven't started that but we want to add Redis Cluster support soon.

Happy to answer any other questions. Let me know when I should merge your
pull requests.

Best,
Felix

Chris Curtin notifications@github.com schrieb am Di., 18. Okt. 2016 um
03:27 Uhr:

Hi is this project still alive?
I tested with sentinel and was able to run as expected with 3 nodes, when
forcing the original master to crash the library connected to the new
master no problems.
Before I refactor all of the tests to support knowing if running in
Sentinel, can you give me some direction about my ideas in the WIP pull
request?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#30 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABgPCipKFcWQU0TrWrodIh3rEzks5Gh9ks5q1CBwgaJpZM4HnTx_
.

@DivineTraube I've added my final test cases and support for Sentinel. I also added a class similar to RedisSentinelConfiguration to allow a similar pattern for standalone Redis. I also added some example Redis configuration files and instructions to setup a simple local Redis Sentinel.

Please review and let me know if you have questions. otherwise I think I'm done.

Thanks

Chris

Hi, The team working with my changes also added support for multiple databases since our environment requires it. Can you review and accept so we don't have to maintain a separate branch?

Thanks

Chris

We have implemented a test setup for our Jenkins and made some Refactoring of the RedisPool creation.
It is merged and released right now.

Regards