Remove configuration of global pools #149
Comments
|
I like the renames. I think task / operation could also be the other way around, so the new names are better. I don't know anything about logging, but a general hook mechanism might come in very handy in the future. Perhaps abstract hooking from logging. Then we could have things like concurrency profiling. |
|
@chrisseaton there is simple general logging added, I needed something for actors. It's just a lambda taking What type of hooks do you have in mind? |
|
Something that gets a notification when an actor is created, when a message is sent, when it's processed. That sort of thing. But I don't really know much about actor models so it was just an off-hand remark really. |
|
It was a good remark I am logging level very similar events on DEBUG level. It would be better to make it general not just for logger as you suggested. TODO made. |
|
|
I mentioned We could also rename all pools to executor for consistency as well. logging: agree, fyi I've added few debug log calls also to other parts of the gem to log otherwise ignored exceptions. (Look for log method calls.) |
|
We should determine what configuration should be chosen for global pools. Currently we have: def new_task_pool
Concurrent::ThreadPoolExecutor.new(
min_threads: [2, Concurrent.processor_count].max,
max_threads: [20, Concurrent.processor_count * 15].max,
idletime: 2 * 60, # 2 minutes
max_queue: 0, # unlimited
overflow_policy: :abort # raise an exception
)
end
def new_operation_pool
Concurrent::ThreadPoolExecutor.new(
min_threads: [2, Concurrent.processor_count].max,
max_threads: [2, Concurrent.processor_count].max,
idletime: 10 * 60, # 10 minutes
max_queue: [20, Concurrent.processor_count * 15].max,
overflow_policy: :abort # raise an exception
)
endSince the operation_pool is intended for longer and blocking tasks it may deadlock easily, e.g. when all threads are waiting for an event scheduled for execution in queue and there is no free thread. I would increase the |
|
I am thinking about following for the new configuration: module Concurrent
# executors do not allocate the threads immediately so they can be constants
# all thread pools are configured to never reject the job
# TODO optional auto termination
module Executors
IMMEDIATE_EXECUTOR = ImmediateExecutor.new
# Only non-blocking and short tasks can go into this pool, otherwise it can starve or deadlock
FAST_EXECUTOR = Concurrent::FixedThreadPool.new(
[2, Concurrent.processor_count].max,
8000
idletime: 60, # 1 minute same as Java pool default
max_queue: 0 # unlimited
)
# IO and blocking jobs should be executed on this pool
IO_EXECUTOR = Concurrent::ThreadPoolExecutor.new(
min_threads: [2, Concurrent.processor_count].max,
max_threads: Concurrent.processor_count * 100,
idletime: 60, # 1 minute same as Java pool default
max_queue: 0 # unlimited
)
def executor(which)
case which
when :immediate, :immediately
IMMEDIATE_EXECUTOR
when :fast
FAST_EXECUTOR
when :io
IO_EXECUTOR
when Executor
which
else
raise TypeError
end
end
end
extend Executors
endWhat do you think? |
|
Thank you @pitr-ch for this. The original configuration was very arbitrary based on a few internet searches I did. It was not based on actual use of the gem. Now that we're growing a community of users I agree that we should evolve the global configuration to support them better. I am comfortable with this configuration. |
|
Addressed in PR #255. |
Concurrentspace@chrisseaton @jdantonio, I've tried to capture the result of the discussion.
The text was updated successfully, but these errors were encountered: