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

Release db connections after each callback. #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hebnern
Copy link

@hebnern hebnern commented Mar 27, 2014

Releases db connections after each callback is called so that a small number of db connections can be shared by a larger number of websocket connections. Proposed fix for #11

@ngauthier
Copy link
Owner

So, do you have to check one out each time, or does rails check it out when you make the call?

I get what you were saying now in the other issue.

If there are a lot of high activity frequent connections there's still gonna be a problem. I don't have a project I'm using tubesock on right now, have you run this code much on your own app?

@hebnern
Copy link
Author

hebnern commented Mar 27, 2014

Yes, rails will check out a new connection when you do any ActiveRecord operations that need to go to the db.

I ran it last night quickly. Not with heavy load, but it didn't crash at least.

@ngauthier
Copy link
Owner

OK. When you said "do this for the user when we know they're done" I was thinking after close, not after all the callbacks. I get it now.

Would you mind running the code a bit, then pinging me here later? We don't have a lot of tests for this sort of thing.

@teddythetwig
Copy link
Contributor

I can submit a PR for this if people are still interested in the feature. Currently, we just wrap all code that uses ActiveRecord with ActiveRecord::Base.connection_pool.with_connection &block. The problem is that just clearing connections after running your handlers isn't necessarily sufficient, because you can still checkout a connection inside the hijack block, and you will have to manually clear that connection.

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

Successfully merging this pull request may close these issues.

3 participants