Skip to content

Make threadpools optional for sharded/size_diff storage

Jeremiah Bonney requested to merge jbonney/make-optional-threadpool into master

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.

Merge request reports