From e5c8a79e041d3fe4fddae4bcc257b02464adbd17 Mon Sep 17 00:00:00 2001 From: Daniel Garnier-Moiroux Date: Tue, 17 Dec 2024 22:28:56 +0100 Subject: [PATCH] tests: enable parallel testing to speed up database-backed tests - Now that we have re-enabled DB testing with gh-3186, Postgres and MySQL tests are slower, and it is useful to enable some level of parallelism. --- build.gradle | 1 - docs/testing.md | 50 ++++++++++++++++++- gradle.properties | 1 + scripts/docker-compose.yml | 4 ++ scripts/mysql/init-db.sh | 3 +- scripts/postgresql/init-db.sh | 3 +- scripts/start_db_helper.sh | 3 +- .../identity/uaa/db/UaaDatabaseName.java | 1 + uaa/build.gradle | 13 +++++ 9 files changed, 71 insertions(+), 8 deletions(-) diff --git a/build.gradle b/build.gradle index c42a889e79e..1a93e85c056 100644 --- a/build.gradle +++ b/build.gradle @@ -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 diff --git a/docs/testing.md b/docs/testing.md index f1445d89bf2..763433118d5 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -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. \ No newline at end of file +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`. \ No newline at end of file diff --git a/gradle.properties b/gradle.properties index ce4c5213511..e8f382a981e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -12,3 +12,4 @@ ossrhUsername= ossrhPassword= group=org.cloudfoundry.identity archivesBaseName="uaa" +org.gradle.workers.max=6 \ No newline at end of file diff --git a/scripts/docker-compose.yml b/scripts/docker-compose.yml index 067f338be39..77a26251f9a 100644 --- a/scripts/docker-compose.yml +++ b/scripts/docker-compose.yml @@ -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: diff --git a/scripts/mysql/init-db.sh b/scripts/mysql/init-db.sh index 274a419e64a..2b4fbbd0835 100755 --- a/scripts/mysql/init-db.sh +++ b/scripts/mysql/init-db.sh @@ -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() { diff --git a/scripts/postgresql/init-db.sh b/scripts/postgresql/init-db.sh index 2f65fcb7beb..bb14f785f8f 100755 --- a/scripts/postgresql/init-db.sh +++ b/scripts/postgresql/init-db.sh @@ -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() { diff --git a/scripts/start_db_helper.sh b/scripts/start_db_helper.sh index 64d36437dbf..6e23284f440 100755 --- a/scripts/start_db_helper.sh +++ b/scripts/start_db_helper.sh @@ -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() { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/db/UaaDatabaseName.java b/server/src/main/java/org/cloudfoundry/identity/uaa/db/UaaDatabaseName.java index 105fa5f140e..7d1418490a4 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/db/UaaDatabaseName.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/db/UaaDatabaseName.java @@ -14,6 +14,7 @@ public String getName() { return UAA_DB_NAME; } + System.out.println("⚙️ daniel --- " + gradleWorkerId); return UAA_DB_NAME + "_" + gradleWorkerId; } } diff --git a/uaa/build.gradle b/uaa/build.gradle index 0cf7ebe7b49..2d1742cb897 100644 --- a/uaa/build.gradle +++ b/uaa/build.gradle @@ -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 {