Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add connection pooling #26

Open
dsully opened this issue Dec 13, 2012 · 11 comments
Open

Add connection pooling #26

dsully opened this issue Dec 13, 2012 · 11 comments
Labels

Comments

@dsully
Copy link

dsully commented Dec 13, 2012

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

Thanks

@earl
Copy link
Owner

earl commented Jan 8, 2013

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
Copy link
Author

dsully commented Jan 8, 2013

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
Copy link
Owner

earl commented Jan 9, 2013

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

@dsully
Copy link
Author

dsully commented Jan 9, 2013

Yes, beaneater.

@Doerge
Copy link
Contributor

Doerge commented Jun 23, 2013

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
Copy link

ysmolski commented Nov 6, 2013

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
Copy link

ysmolski commented Nov 6, 2013

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

@earl
Copy link
Owner

earl commented Nov 6, 2013

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
Copy link
Contributor

Doerge commented Nov 6, 2013

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
Copy link

ysmolski commented Nov 8, 2013

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
Copy link
Contributor

Doerge commented Nov 8, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants