Skip to content

Raise error when DB pool size is shrunk

Matthias Käppler requested to merge 35170-min-pool-size-check into master

What does this MR do?

Amendment to #35170 (closed)

Due to a problem in initialization order, we had ended up setting the Geo DB pool size to 1 instead of max_threads, since the code path that performs the adjustment was never hit.

This change introduces an additional sanity check which makes sure that neither the main DB nor the Geo DB the pool size ever shrinks after the initializer has run (i.e. it fails fast during app initialization should that happen.)

One problem here is that these checks only really make sense if database re-configuration actually happens. However, that DB configuration did not happen was the crux of the original problem, so we have a bit of a catch 22 situation here. To alleviate that, I changed the check for when Geo config should be adjusted from "does any DB config exist" to "is Geo enabled" (Geo.enabled?) which should be true or false regardless of the order of initializers. (changed it back after it was debated to yield the right behavior.)

I also extracted the setup code into 2 helper methods, since this was all getting a little unwieldy.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

The expectation here is that any change applied in the initializer that results in a pool size smaller than what it was before, will crash the app on purpose (as that should never happen and should never go unnoticed.)

Contrary to initial considerations, since we're not checking for particular values but just a "less than" relation, it should be OK to perform these checks in any environment, including development (where folks often set values that would not make sense in production, such as pool = 1.)

I am still unclear on how to test any sort of Geo related change. I need help with this.

Edited by 🤖 GitLab Bot 🤖

Merge request reports