Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: enable parallel testing to speed up database-backed tests #3203

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ subprojects {
[compileJava, compileTestJava]*.options*.compilerArgs = ["-Xlint:none", "-nowarn"]

test {
maxParallelForks = 1
// when failFast = true AND retry is on, there is a serious issue:
// gradle might stop the test run due to the failFast but still concludes with BUILD SUCCESSFUL (if the retry is successful)
failFast = false
Expand Down
50 changes: 49 additions & 1 deletion docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,52 @@ If you have timing-based issues in your test, ensure that you set `$TZ` before s

$ TZ="Europe/Paris" docker compose up

It is not required, and MySQL will default to using UTC.
It is not required, and MySQL will default to using UTC.

## Running tests in parallel

Depending on the machine used for tests, HSQL-based tests are twice as fast as Postgres or MySQL tests. The bottleneck
project is `cloudfoundry-identity-uaa`. Tests for other projects are much faster, and since each project is tested in
parallel, they all complete before the longer `cloudfoundry-identity-uaa` test is over.

While it makes sense to run tests in parallel for `cloudfoundry-identity-uaa` when testing locally, it has a few
consequences. First, it might bring it test pollution, see [above](#test-pollution). Second, the databases must be able
to handle the incoming connections, but this is handled by setting the number of concurrent connections to 250 on both
Postgres and MySQL.

Furthermore, too many tests running in parallel has diminishing returns, because the Spring testing support caches
ApplicationContext between tests to avoid bootstrapping too many contexts. Having many tests in parallel means there
will be a lot of cache misses, and every gradle worker handling a test will have to bootstrap multiple
ApplicationContext, which is usually quite slow. A value that has been previously used is 6 workers in parallel, and,
for any given project, 4 tests in parallel. Since `cloudfoundry-identity-uaa` is the bottleneck, we make its tests run
in parallel, and keep other projects running sequential tests. Empirically, this yields a x2 or more speedup on the test
suite on a powerful developer machine. Increasing parallelism further yields no benefits (for example, tests are
slightly slower on a Mac M1 using 6 tests in parallel).

Technically, parallelism is controlled by two parameters:

- `org.gradle.workers.max` in `gradle.properties`, set to 6. This means that there are no more than 6 gradle processes
running at any given time. It does not have anything to do with tests directly. This might mean you have a 3 compile
processes for 3 different projects, as well as 3 test processes for different projects. Within a given project, by
default, tests are running sequentially and never in parallel. Unless you configure parallelism for tests, see below:
- `maxParallelForks` in the test task in the project's `build.gradle`, set to 4. This controls how many tests can run in
parallel within a given project. It is bounded by the max workers set above - so if there are 5 compile tasks running,
and one test task, the test task will run tests sequentially as there is a single worker available left.

With this setup, we get 11 test workers total, 4 for `cloudfoundry-identity-uaa`, plus 1 per remaining project:

- `cloudfoundry-identity-metrics-data`
- `cloudfoundry-identity-statsd-lib`
- `cloudfoundry-identity-model`
- `cloudfoundry-identity-statsd`
- `cloudfoundry-identity-samples:cloudfoundry-identity-api`
- `cloudfoundry-identity-samples:cloudfoundry-identity-app`
- `cloudfoundry-identity-server`

Since `cloudfoundry-identity-uaa` depends on all the projects, it runs last, and gets workers 8, 9, 10 and 11. If some
tests did not need to be re-run because the code hasn't changed in some projects, it may get assigned workers with lower
IDs. This means that we need all databases between `uaa_1` and `uaa_11`, included. To make sure we have enough room for
future changes, we will keep the current setup with 24 databases, see [test pollution](#test-pollution).

In single-CPU scenarios (i.e. in CI), we would only need two databases, `uaa_7` for`cloudfoundry-identity-server` and
`uaa_8` for `cloudfoundry-identity-uaa`.
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ ossrhUsername=
ossrhPassword=
group=org.cloudfoundry.identity
archivesBaseName="uaa"
org.gradle.workers.max=6
4 changes: 4 additions & 0 deletions scripts/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ services:
size: 1073741824 # 1024 * 2^20 bytes = 1024Mb
environment:
- POSTGRES_PASSWORD=changeme
command:
- postgres
- -c
- max_connections=250
mysql:
image: "mysql:8"
ports:
Expand Down
3 changes: 1 addition & 2 deletions scripts/mysql/init-db.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

set -euo pipefail

# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
# We make extra dbs because a gradle worker ID can exceed the max number of workers.
# See docs/testing.md, sections on "test pollution" and "parallelism"
NUM_OF_DATABASES_TO_CREATE=24

function initDB() {
Expand Down
3 changes: 1 addition & 2 deletions scripts/postgresql/init-db.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

set -euo pipefail

# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
# We make extra dbs because a gradle worker ID can exceed the max number of workers.
# See docs/testing.md, sections on "test pollution" and "parallelism"
NUM_OF_DATABASES_TO_CREATE=24

function initDB() {
Expand Down
3 changes: 1 addition & 2 deletions scripts/start_db_helper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
set -eu
script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
# We make extra dbs because a gradle worker ID can exceed the max number of workers.
# See docs/testing.md, sections on "test pollution" and "parallelism"
NUM_OF_DATABASES_TO_CREATE=24

function createDB() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ public String getName() {

return UAA_DB_NAME + "_" + gradleWorkerId;
}
}
}
13 changes: 13 additions & 0 deletions uaa/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ test {
exclude("**/*IT.class")
exclude("**/*Docs.class")
systemProperty("mock.suite.test", "true")

// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Running tests in parallel
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

// Count available cores. We assume 2 logical cores per physical core.
// In case there is only one vCPU, we count 1 full core.
var availableCpus = Math.max(Runtime.getRuntime().availableProcessors() / 2, 1.0)

// We want some amount of parallelism, but it does not make sense to run too many
// tests in parallel, see docs/testing. We target 4 tests in parallel at most. If
// there are less CPUs available, we use all available CPUs but no more.
maxParallelForks = Math.min(availableCpus, 4)
}

task populateVersionfile {
Expand Down
Loading