aws / aws-sdk-rails

Official repository for the aws-sdk-rails gem, which integrates the AWS SDK for Ruby with Ruby on Rails.
Other
596 stars 62 forks source link

Clean shutdown procedure is not invoked when the main thread participates in thread pool execution #123

Closed pjurewicz closed 5 months ago

pjurewicz commented 5 months ago

Concurrent::ThreadPoolExecutor is initialized with fallback_policy: :caller_runs This means that the work will be executed in the thread of the caller, instead of being given to another thread in the pool when the queue of waiting work has reached the maximum size (backpressure) and no new threads can be created. In such case all the exceptions are rescued in the RubyThreadPoolExecutor#run_task. This means that Aws::Rails::SqsActiveJob#run rescue block is not triggered and clean shutdown procedure is not invoked.

jterapin commented 5 months ago

Thanks for opening the ticket. We will take a look and get back to you!

alextwoods commented 5 months ago

Thanks for submitting this - good catch. I've had a TODO in the code for awhile to fix the use of caller_runs for backpressure. I'll take a look at fixing that.

mostlyobvious commented 5 months ago

One more issue with main thread acting as an additional worker — it needs additional connection to the pooled resources over chosen thread count. For example with explicitly chosen 10 worker threads, the database connection pool would have to be 10 + 1 in order to not experience ActiveRecord::ConnectionTimeoutError when backpressure limit is reached.

This is not immediately obvious until experiencing it and reading the code.

alextwoods commented 5 months ago

Yeah this is a good point as well. I had always intended to revisit the use of caller_runs. I believe #124 should make the resource usage more explicit as well.