Handle graceful shutdown of ConsumerWorkPool when canceling a consumer #437
Conversation
| @@ -17,9 +17,12 @@ class ConsumerWorkPool | |||
| attr_reader :size | |||
| attr_reader :abort_on_exception | |||
|
|
|||
| def initialize(size = 1, abort_on_exception = false) | |||
| def initialize(size = 1, abort_on_exception = false, shutdown_timeout = nil) | |||
There was a problem hiding this comment.
I think it's perfectly fine to have a non-nil default timeout.
| @@ -337,14 +337,14 @@ def transport_write_timeout | |||
| # opened (this operation is very fast and inexpensive). | |||
| # | |||
| # @return [Bunny::Channel] Newly opened channel | |||
| def create_channel(n = nil, consumer_pool_size = 1, consumer_pool_abort_on_exception = false) | |||
| def create_channel(n = nil, consumer_pool_size = 1, consumer_pool_abort_on_exception = false, consumer_pool_shutdown_timeout = nil) | |||
There was a problem hiding this comment.
Again, a non-nil timeout (maybe up to 60 seconds?) sounds OK and would make most people benefit from this improvement.
There was a problem hiding this comment.
Ok sounds good, I wasn't sure so left it off but I think this makes sense :)
|
It looks good at first glance. I'd recommend using a timeout by default, e.g. the same the Java client uses. |
|
Changed to use a 60s timeout as mentioned, I wanted to not change behavior but after you mentioned it I think it makes sense to be safe by default. |
|
@michaelklishin any more thoughts? |
|
Thank you. |
|
Thanks, I will update the documentation when I get a spare minute through the week. |
In ruby-amqp/bunny#382 and ruby-amqp/bunny#437 `Session#create_channel` got some additional arguments. This change allows calling of `create_channel` with additional arguments.
In ruby-amqp/bunny#382 and ruby-amqp/bunny#437 `Session#create_channel` got some additional arguments. This change allows calling of `create_channel` with additional arguments.
In ruby-amqp/bunny#382 and ruby-amqp/bunny#437 `Session#create_channel` got some additional arguments. This change allows calling of `create_channel` with additional arguments.
This carries on the work of #341 to implement this feature fully. You can now configure a worker pool shutdown timeout when creating the channel (off by default).
Then upon processing of the basic_cancel instead of killing the worker pool as previously done, the worker pool will be shut down and instructed to wait for any workers to complete (within the timeout period).
This allows for a consumer to stop receiving new messages but still be able to complete any existing work before the worker pool is shut down.
All existing behavior remains in the case of closing a connection, we will not wait for workers as the worker might need to ack the message and in this scenario the connection has been closed and this would not be possible.
I will update the documentation as a seperate PR.