-
Notifications
You must be signed in to change notification settings - Fork 43
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
Each web socket holds a separate db connection #11
Comments
Agreed. This is a deal breaker for me. I am currently using faye which uses a single thread running an EventMachine Reactor to handle all IO, so just one db connection is needed for all websockets. However I really love how tubesock integrates with rails versus the low level rack integration that faye requires. Plus I am having trouble with faye because its EM thread doesn't release db connections if the thread is killed by an exception. Tubesock on the other hand properly cleans up after itself. I am thinking of either hacking EM into tubesock or rails integration + ActiveRecord cleanup similar to tubesock into faye. Hmmm... edit: On second thought, we could just return the connection to the connection pool after the onopen / onmessage / onclose events. |
You can always checkin the current database connection back to the pool and only checkout one if needed. So no need for EM or Faye or whatnot. |
@tbuehlmann yup, that's what tubesock does, pretty much. @hebnern the main reason I came up with tubesock is because I didn't want to use EM. Because EM can only process one request at a time, a long running request stops short ones from processing. Tubesock threads, so a long one can start, and while it's running lots of short ones can complete. Also, it's actually Rails that checks out a db connection per request. Tubesock simply makes sure they get cleaned up when the connection closes. That's one of the nice things about tubesock, is that is leverages existing Rails awesomeness. |
I think @fokcep means that active connections still hold a DB connection, even if not needed. And one simple answer to that would be to checkin the connection manually. |
Yup. also, this code only occurs if you use tubesock/hijack for rails. Ideally plain tubesock would not do this, and you could hijack yourself if you wanted a slightly different behavior. The hijack code is actually really small, so it would not be particularly difficult to get your own behavior: https://github.com/ngauthier/tubesock/blob/master/lib/tubesock/hijack.rb IMO I think the fact that it checks out a connection is a good thing, because it makes it easier to use, even if it hurts scalability. If you want to scale it up you can change the behavior a bit. Also, I'd be interested to see memory consumption if you set your max db connections to something silly like 5,000. How much does it hurt? |
I agree that it is nice that connections are checked out automatically, but I don't think jacking up max db connections is a great solution. Since we already have points where we know that the thread is "done" with its connection (after we call the callback), why not just release its connection at that point, and let it check out a new one next time it needs it? Then we share a small number of db connections between all threads. I threw something like this together last night on my branch. Pull Request: #24 |
If you ever know you're done with the DB, but you're keeping the connection open for longer, just run this line: https://github.com/ngauthier/tubesock/blob/master/lib/tubesock/hijack.rb#L33 (you don't need the |
Yep, but why don't we just do this for the user when we know they are done? |
How do you know the user is done? |
We're talking on this at #24 too. He means after a callback is executed the user's code is not run until the next callback, so clean db connections after callbacks. |
This commit will cause the socket to close if a message handler receives an error. Test included
Would be more scalable not to hold a db connection and pass it with the block
The text was updated successfully, but these errors were encountered: