earl / beanstalkc

A simple beanstalkd client library for Python
Apache License 2.0
457 stars 115 forks source link

Add connection pooling #26

Open dsully opened 11 years ago

dsully commented 11 years ago

Any interest in merging pooled connection support if I were to implement it?

Thanks

earl commented 11 years ago

In principle, yes. Even more so if it can be kept as simple as possible. Could you shortly describe how you envision it behaving, in rough terms?

dsully commented 11 years ago

I was looking at doing a rough port of how the Ruby client works - some operations read from all hosts in the pool & collate the results, other operations work against a specific host.

I also prefer simple - and am thinking of a separate class BeanstalkPool or similar, but there might need to be some changes to the core code.

Thanks

earl commented 11 years ago

I assume you are talking of beaneater? I'll have a look again at how they do pooling.

dsully commented 11 years ago

Yes, beaneater.

Doerge commented 11 years ago

Hi there.

As part of my job I forked and wrote a simple Pool-class. I haven't looked at beaneater, but it sounds like I'm doing exactly what you talk about. I haven't made a pull-request yet because I thought we would come across various issues while coding up against the pool, and I have managed to catch a bug or two this way. I will make a pull-request when I feel it is deserves to be a part of this great client!

ysmolski commented 10 years ago

I think having pool is essential for such a client. I have noticed that Doerge has made additions with Pool class [1]. Earl, what do you think about merging into your original library?

  1. https://github.com/Doerge/beanstalkc
ysmolski commented 10 years ago

Doerge, do you mind if I work on your fork with Pooled connections to cut some rough edges? :-)

earl commented 10 years ago

If someone prepares a pull-request, I'll be happy to review it. Doerge mentioned that he came across various issues he wanted to shake out first; I don't know what the status of that is.

Doerge commented 10 years ago

Hiya.

We are using my branch in production and it has been a while since I came across any issues. I don't know if that justifies a pull-request? (I'm new to github'ish..)

@ysmolsky Fork away! I would love to see some polish! For the sake of my coding-bettering what rough edges have you noticed?

edit: I logged 3 issues with the pool btw: https://github.com/Doerge/beanstalkc/issues?state=open

ysmolski commented 10 years ago

Well, the polish I meant is about code style mostly. But more importantly is that reconnection does not work as expected for your fork, b/c of issues you mentioned by that link.

For example the most obvious is the we need to reuse and rewatch all those tubes we were using and watching prior the disconnect. I am going to implement them in fork with BlockingConnection class...

Also I don't really like the idea of having multiple beastalkd server and pulling reserve() from random instances. Thinking about having something more reliable..

Doerge commented 10 years ago

Cool! I'm looking forward to learn from your polishing!

@reconnecting Cool! I'll keep an eye on your fork :)

@reserving from random instances: Yeah I did consider peeking at the different bstalkds but for our scenario I went with the simple solution at first. Setting a time out on the reserve and then trying the next in line should be a better solution.