ngauthier / tubesock

Websocket interface on Rack Hijack w/ Rails support
MIT License
620 stars 43 forks source link

Each web socket holds a separate db connection #11

Open fokcep opened 11 years ago

fokcep commented 11 years ago

Would be more scalable not to hold a db connection and pass it with the block

hebnern commented 10 years ago

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.

tbuehlmann commented 10 years ago

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.

ngauthier commented 10 years ago

@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.

tbuehlmann commented 10 years ago

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.

ngauthier commented 10 years ago

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?

hebnern commented 10 years ago

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

ngauthier commented 10 years ago

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 if defined? ActiveRecord part)

hebnern commented 10 years ago

Yep, but why don't we just do this for the user when we know they are done?

tbuehlmann commented 10 years ago

How do you know the user is done?

ngauthier commented 10 years ago

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.