Make threadpools optional for sharded/size_diff storage
Before raising this MR, consider whether the following are required, and complete if so:
-
Unit tests -
Metrics -
Documentation update(s)
If not required, please explain in brief why not.
Description
The ThreadPoolExecutor
used by ShardedStorage
and SizeDifferentiatedStorage
currently default to 2x the number of configured shards/storages, but this is not a great default. Since the ThreadPoolExecutors
are storage-wide, this can result in many parallel requests being blocked on the small amount of threads used by these storages. This makes it hard to compose these storages, which is a bit counter-productive to BuildGrid's storage philosophy. This PR updates these storages to only create a ThreadPoolExecutor
if thread-pool-size
is explicitly specified.
The code for this ended up being pretty clean since a map
can be used in both cases, either the standard one or ThreadPoolExecutor.map
, so the flow looks almost identical regardless of whether a ThreadPoolExecutor
is being used or not.
As part of these changes I also updated ShardedStorage
to bring it more inline with SizeDifferentiatedStorage
, so now the two storages are almost identical.