GirlFriday::WorkQueues never shut down

[emschwar]

There doesn't seem to be a way to shut down the GirlFriday::WorkQueue that gets started in Blather::Client#initialize. For a long-running process that starts/stops plenty of connections, this means you end up with a lot of threads just hanging out, but doing no work.

I have a few of ideas, and wanted to discuss them before developing a patch:

1. Make queues entirely optional -- say, pass a :queue_size => 0 or some such to the client initializer, and just never setting @handler_queue in that case (we'd obviously have to tweak #receive_data to process things immediately in that case)
2. Make Blather::Client::#close_connection shutdown the work queue, and #run would then have to re-create it (or better yet, move the work queue creation into the #run method, since there's no point in having any queues until you're doing something with them.
3. Blather could have a shared work queue that all clients drew from-- just make @handler_queue into @@handler_queue and don't initialize it if some other client has done so. We could size the queue with something like Blather::Client.handler_queue_size = 60 or some such.

I have a moderate preference for option number 3, but if those with more experience on the project disagree, I don't mind a different patch strategy, just so long as those threads don't live on forever.

[benlangfeld]

I'm happy with a combo of 1 + 2. I'm not a big fan of globals as in #3, though if you can defend your preference I'm willing to be convinced.

[emschwar]

My reasoning is mostly that in my particular application, I tend to have a large number of clients (20k+), mostly doing nothing. I don't need anywhere near 5 worker threads per client-- most of the time I don't need any, which is why I'd be happy with a shared pool.

[emschwar]

I'm aware my needs are probably somewhat idiosyncratic, which is why I provided options, though. :)

[benlangfeld]

Would skipping the worker pool entirely suit your use case?

[emschwar]

I'm not sure-- I want to try it first. I think it might be okay, but I'm not positive how it'll behave under load. How about I'll prototype the patch using 1+2, and if it works for me, I'll push it up.