diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 30d5754e27a..077f24a48c4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -206,6 +206,10 @@ jobs: -x :catalogs-contrib:catalog-jdbc-clickhouse:test -x :catalogs-contrib:catalog-jdbc-hologres:test -x :catalogs-contrib:catalog-jdbc-oceanbase:test + # Core's tests run in the core-test job, one job per core test task. + -x :core:test + -x :core:coreMySQLTest + -x :core:corePostgreSQLTest ) if [ "${{ needs.changes.outputs.mcp_server_changes }}" != "true" ]; then @@ -223,13 +227,244 @@ jobs: ./gradlew "${gradle_args[@]}" - - name: Fetch base branch for coverage diff + - name: Upload coverage data if: github.event_name == 'pull_request' + uses: actions/upload-artifact@v7 + with: + name: jacoco-build + # build.gradle.kts anchors the artifact at the repository root, so the report paths keep + # their module prefix, which jacoco_report.py uses to name modules. + path: | + build.gradle.kts + **/build/reports/jacoco/test/jacocoTestReport.xml + if-no-files-found: ignore + retention-days: 1 + + - name: Upload unit tests report + uses: actions/upload-artifact@v7 + if: failure() + with: + name: unit test report + path: | + build/reports + catalogs-contrib/**/*.log + catalogs-contrib/**/*.tar + catalogs/**/*.log + catalogs/**/*.tar + + core-test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + java-version: [ 17 ] + # One job per core test lane, so each lane's results and coverage inputs are inspectable on + # their own. Lanes are defined in core/build.gradle.kts. + lane: [ h2, mysql, postgresql ] + include: + - lane: h2 + task: test + skip-docker-tests: true + - lane: mysql + task: coreMySQLTest + skip-docker-tests: false + - lane: postgresql + task: corePostgreSQLTest + skip-docker-tests: false + timeout-minutes: 120 + needs: changes + if: >- + needs.changes.outputs.source_changes == 'true' && + needs.changes.outputs.maintenance_module_only_changes != 'true' + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/setup-java-toolchains + with: + java-version: ${{ matrix.java-version }} + + - name: Free up disk space + run: | + dev/ci/util_free_space.sh + + - name: Test database concurrency build logic + id: buildsrc-tests + if: matrix.lane == 'h2' + run: ./gradlew -p buildSrc test + + - name: Upload database concurrency build logic test report + if: ${{ failure() && steps.buildsrc-tests.outcome == 'failure' }} + uses: actions/upload-artifact@v7 + with: + name: core-database-buildsrc-test-report + path: | + buildSrc/build/reports/tests/test + buildSrc/build/test-results/test + if-no-files-found: warn + retention-days: 1 + + - name: Run core ${{ matrix.lane }} lane + run: | + ./gradlew :core:${{ matrix.task }} \ + -PskipWeb=true \ + -PskipITs \ + -PskipDockerTests=${{ matrix.skip-docker-tests }} + + - name: Validate core test evidence + run: | + junit_xml="$(find "core/build/test-results/${{ matrix.task }}" \ + -type f -name 'TEST-*.xml' -print -quit)" + test -n "${junit_xml}" + test -s "${junit_xml}" + test -s "build/reports/tests/core/${{ matrix.task }}/index.html" + test -s "core/build/jacoco/${{ matrix.task }}.exec" + if [ "${{ matrix.lane }}" != "h2" ]; then + shard_record="$(find \ + "core/build/test-results/${{ matrix.task }}/shard-telemetry" \ + -type f -name 'worker-*.log' -size +0c -print -quit)" + test -n "${shard_record}" + test -s "${shard_record}" + fi + + - name: Upload core test evidence + uses: actions/upload-artifact@v7 + with: + name: core-${{ matrix.lane }}-test-evidence + path: | + core/build/test-results/${{ matrix.task }} + build/reports/tests/core/${{ matrix.task }} + core/build/jacoco/${{ matrix.task }}.exec + if-no-files-found: error + retention-days: 1 + + - name: Upload core test report + uses: actions/upload-artifact@v7 + if: failure() + with: + name: core test report ${{ matrix.lane }} + path: | + build/reports/tests/core + core/build/test-results + + # The same MySQL and PostgreSQL tasks against a database the job provides as a service + # container, the way a Kubernetes pod would use a sidecar. Gradle starts no container here. + core-test-injected: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - backend: mysql + task: coreMySQLTest + image: mysql:8.0 + port: 3306 + url: jdbc:mysql://127.0.0.1:3306/?useSSL=false&allowPublicKeyRetrieval=true + health: --health-cmd="mysqladmin ping -h 127.0.0.1 -proot" --health-interval=5s --health-timeout=5s --health-retries=30 + - backend: postgresql + task: corePostgreSQLTest + image: postgres:13 + port: 5432 + url: jdbc:postgresql://127.0.0.1:5432/postgres + health: --health-cmd="pg_isready -U root" --health-interval=5s --health-timeout=5s --health-retries=30 + timeout-minutes: 120 + needs: changes + if: >- + needs.changes.outputs.source_changes == 'true' && + needs.changes.outputs.maintenance_module_only_changes != 'true' + services: + db: + image: ${{ matrix.image }} + env: + MYSQL_ROOT_PASSWORD: root + POSTGRES_USER: root + POSTGRES_PASSWORD: root + ports: + - ${{ matrix.port }}:${{ matrix.port }} + options: ${{ matrix.health }} + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/setup-java-toolchains + with: + java-version: 17 + + - name: Run core ${{ matrix.backend }} tests against the injected database + env: + GRAVITINO_TEST_MYSQL_URL: ${{ matrix.backend == 'mysql' && matrix.url || '' }} + GRAVITINO_TEST_POSTGRESQL_URL: ${{ matrix.backend == 'postgresql' && matrix.url || '' }} + GRAVITINO_TEST_MYSQL_USER: root + GRAVITINO_TEST_MYSQL_PASSWORD: root + GRAVITINO_TEST_POSTGRESQL_USER: root + GRAVITINO_TEST_POSTGRESQL_PASSWORD: root + run: | + ./gradlew :core:${{ matrix.task }} -PskipWeb=true -PskipITs -PskipDockerTests=true \ + | tee core-injected.log + + - name: Check the tests used the injected database + run: | + grep -q "\[CORE-DB-SOURCE\] task=:core:${{ matrix.task }} backend=${{ matrix.backend }} source=injected" core-injected.log + # Only the service container may exist: Gradle must not have started its own. + test "$(docker ps -aq | wc -l)" -eq 1 + + - name: Upload core injected test report + uses: actions/upload-artifact@v7 + if: failure() + with: + name: core injected test report ${{ matrix.backend }} + path: | + build/reports/tests/core + core/build/test-results + + coverage: + runs-on: ubuntu-latest + timeout-minutes: 30 + needs: [ changes, build, core-test ] + if: >- + always() && + github.event_name == 'pull_request' && + needs.build.result == 'success' && + (needs.changes.outputs.maintenance_module_only_changes == 'true' || + needs['core-test'].result == 'success') + steps: + - uses: actions/checkout@v4 + + - uses: ./.github/actions/setup-java-toolchains + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + with: + java-version: 17 + + - name: Fetch base branch for coverage diff run: git fetch origin ${{ github.base_ref }} --depth=1 + - name: Download coverage data + uses: actions/download-artifact@v4 + with: + pattern: jacoco-* + merge-multiple: true + + - name: Download core test evidence + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + uses: actions/download-artifact@v4 + with: + pattern: core-*-test-evidence + path: core-test-evidence + + - name: Generate combined core coverage + if: needs.changes.outputs.maintenance_module_only_changes != 'true' + run: | + mkdir -p core/build/jacoco + for task in test coreMySQLTest corePostgreSQLTest; do + exec_file="$(find core-test-evidence -path "*/core/build/jacoco/${task}.exec" -print -quit)" + test -n "${exec_file}" + install -m 0644 "${exec_file}" "core/build/jacoco/${task}.exec" + done + ./gradlew \ + :core:jacocoTestReport \ + -PskipWeb=true + test -s core/build/reports/jacoco/test/jacocoTestReport.xml + - name: Generate Coverage Report id: coverage - if: github.event_name == 'pull_request' run: | python3 dev/ci/jacoco_report.py \ --base-ref "${{ github.base_ref }}" \ @@ -240,11 +475,11 @@ jobs: --output coverage-report.md - name: Save PR number - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' run: echo "${{ github.event.pull_request.number }}" > pr-number.txt - name: Upload Coverage Report - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' uses: actions/upload-artifact@v7 with: name: coverage-report @@ -253,19 +488,7 @@ jobs: pr-number.txt - name: Output Coverage Info - if: github.event_name == 'pull_request' && steps.coverage.outputs.has_reports == 'true' + if: steps.coverage.outputs.has_reports == 'true' run: | echo "Total coverage ${{ steps.coverage.outputs.coverage-overall }}" echo "Changed Files coverage ${{ steps.coverage.outputs.coverage-changed-files }}" - - - name: Upload unit tests report - uses: actions/upload-artifact@v7 - if: failure() - with: - name: unit test report - path: | - build/reports - catalogs-contrib/**/*.log - catalogs-contrib/**/*.tar - catalogs/**/*.log - catalogs/**/*.tar diff --git a/build.gradle.kts b/build.gradle.kts index 0161f01f724..95ca5a85a81 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -28,6 +28,8 @@ import com.github.jk1.license.render.InventoryHtmlReportRenderer import com.github.jk1.license.render.ReportRenderer import com.github.vlsi.gradle.dsl.configureEach import net.ltgt.gradle.errorprone.errorprone +import org.apache.gravitino.testing.CoreDatabaseConcurrency +import org.apache.gravitino.testing.SharedDbContainerService import org.apache.tools.zip.ZipEntry import org.apache.tools.zip.ZipOutputStream import org.gradle.api.attributes.java.TargetJvmVersion @@ -81,6 +83,18 @@ val sharedTestEnvironmentLock = gradle.sharedServices.registerIfAbsent( maxParallelUsages.set(1) } +// Owns one shared MySQL container and one shared PostgreSQL container for the whole build (see +// org.apache.gravitino.testing.SharedDbContainerService in buildSrc), so the Core database test +// tasks no longer have to start a fresh container per test-worker fork. Registered here, at the +// root, so a single instance is shared by every project that opts in via `usesService`. Stored on +// rootProject.extra so subproject build scripts (for example core/build.gradle.kts) can look up +// the same registration by name. +val sharedDbContainerService = gradle.sharedServices.registerIfAbsent( + "sharedDbContainerService", + SharedDbContainerService::class +) {} +rootProject.extra["sharedDbContainerService"] = sharedDbContainerService + /** Packages the legal documents for one Maven artifact, retaining dependency provenance. */ @CacheableTask abstract class GenerateJarLegalFiles : DefaultTask() { @@ -458,12 +472,11 @@ allprojects { // Ryuk need privileged mode, if we want to rootless or run non-privileged mode, we need to disable it. param.environment("TESTCONTAINERS_RYUK_DISABLED", "true") - val dockerRunning = project.rootProject.extra["dockerRunning"] as? Boolean ?: false - val macDockerConnector = project.rootProject.extra["macDockerConnector"] as? Boolean ?: false - if (OperatingSystem.current().isMacOsX() && - dockerRunning && - macDockerConnector - ) { + val macDockerConnectorFixedNetwork = + project.rootProject.extra[ + CoreDatabaseConcurrency.MAC_DOCKER_CONNECTOR_FIXED_NETWORK_EXTRA + ] as? Boolean ?: false + if (macDockerConnectorFixedNetwork) { param.environment("NEED_CREATE_DOCKER_NETWORK", "true") } @@ -514,9 +527,14 @@ allprojects { val dockerTest = project.rootProject.extra["dockerTest"] as? Boolean ?: false param.environment("dockerTest", dockerTest.toString()) + // Must stay inside doFirst: tasks such as :core's database lanes set this flag in their own + // configuration action, which may run after this configureEach action registers. + val includeDockerTaggedTests = + param.extensions.extraProperties.properties["includeDockerTaggedTests"] as? Boolean + ?: dockerTest val dorisMultiVersion = project.hasProperty("dorisMultiVersionTest") param.useJUnitPlatform { - if (!dockerTest) { + if (!includeDockerTaggedTests) { excludeTags("gravitino-docker-test") } if (!dorisMultiVersion) { @@ -1017,9 +1035,14 @@ subprojects { val skipTests = project.hasProperty("skipTests") if (!skipTests) { val extraArgs = project.property("extraJvmArgs") as List - jvmArgs = listOf("-Xmx4G") + extraArgs + jvmArgs = listOf(CoreDatabaseConcurrency.TEST_WORKER_MAX_HEAP_ARGUMENT) + extraArgs useJUnitPlatform() - finalizedBy(tasks.getByName("jacocoTestReport")) + val isCoreSuiteTask = + project.path == ":core" && + name in setOf("coreMySQLTest", "corePostgreSQLTest") + if (!isCoreSuiteTask) { + finalizedBy(tasks.getByName("jacocoTestReport")) + } } } @@ -1630,6 +1653,7 @@ project.extra["dockerTest"] = false project.extra["dockerRunning"] = false project.extra["macDockerConnector"] = false project.extra["isOrbStack"] = false +project.extra[CoreDatabaseConcurrency.MAC_DOCKER_CONNECTOR_FIXED_NETWORK_EXTRA] = false // The following is to check the docker status and print the tip message fun printDockerCheckInfo() { @@ -1637,6 +1661,13 @@ fun printDockerCheckInfo() { checkDockerStatus() checkOrbStackStatus() + val macDockerConnectorFixedNetwork = + OperatingSystem.current().isMacOsX() && + (project.extra["dockerRunning"] as? Boolean ?: false) && + (project.extra["macDockerConnector"] as? Boolean ?: false) + project.extra[CoreDatabaseConcurrency.MAC_DOCKER_CONNECTOR_FIXED_NETWORK_EXTRA] = + macDockerConnectorFixedNetwork + val testMode = project.properties["testMode"] as? String ?: "embedded" if (testMode != "deploy" && testMode != "embedded") { return diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts new file mode 100644 index 00000000000..27dfb53bc81 --- /dev/null +++ b/buildSrc/build.gradle.kts @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +plugins { + `java` +} + +repositories { + mavenCentral() +} + +dependencies { + // SharedDbContainerService is a Gradle BuildService. Inside the main build Gradle adds its API + // to buildSrc implicitly; `./gradlew -p buildSrc test` builds buildSrc on its own and needs it. + implementation(gradleApi()) + compileOnly("com.google.code.findbugs:jsr305:3.0.2") + testImplementation("org.junit.jupiter:junit-jupiter-api:5.8.1") + testImplementation("org.junit.jupiter:junit-jupiter-params:5.8.1") + testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.8.1") +} + +tasks.test { + useJUnitPlatform() +} diff --git a/buildSrc/src/main/java/org/apache/gravitino/testing/CoreDatabaseConcurrency.java b/buildSrc/src/main/java/org/apache/gravitino/testing/CoreDatabaseConcurrency.java new file mode 100644 index 00000000000..5470da31558 --- /dev/null +++ b/buildSrc/src/main/java/org/apache/gravitino/testing/CoreDatabaseConcurrency.java @@ -0,0 +1,502 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +import javax.annotation.Nullable; + +/** Calculates a conservative number of forks for the Core database test tasks. */ +public final class CoreDatabaseConcurrency { + + private static final long MIB = 1024L * 1024L; + private static final long TEST_WORKER_MAX_HEAP_MIB = 4096L; + private static final int ROLLOUT_CAP = 2; + private static final int DEFAULT_FORKS = 1; + + /** Maximum-heap JVM argument configured for every Gradle test worker. */ + public static final String TEST_WORKER_MAX_HEAP_ARGUMENT = + "-Xmx" + TEST_WORKER_MAX_HEAP_MIB + "m"; + + /** Maximum heap configured for every Gradle test worker. */ + public static final long TEST_WORKER_MAX_HEAP_BYTES = TEST_WORKER_MAX_HEAP_MIB * MIB; + + /** Default native-memory and database-container allowance for each test worker. */ + public static final long DEFAULT_TEST_WORKER_OVERHEAD_BYTES = 2048L * MIB; + + /** Default memory kept for the Gradle daemon and other build processes. */ + public static final long DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES = 4096L * MIB; + + /** Gradle property for the build-process memory reserve, in MiB. */ + public static final String BUILD_MEMORY_RESERVE_PROPERTY = "coreDatabaseBuildMemoryReserveMiB"; + + /** Environment variable for the build-process memory reserve, in MiB. */ + public static final String BUILD_MEMORY_RESERVE_ENVIRONMENT_VARIABLE = + "CORE_DATABASE_BUILD_MEMORY_RESERVE_MIB"; + + /** Gradle property for native and container memory overhead per test worker, in MiB. */ + public static final String FORK_MEMORY_OVERHEAD_PROPERTY = "coreDatabaseForkMemoryOverheadMiB"; + + /** Environment variable for native and container memory overhead per test worker, in MiB. */ + public static final String FORK_MEMORY_OVERHEAD_ENVIRONMENT_VARIABLE = + "CORE_DATABASE_FORK_MEMORY_OVERHEAD_MIB"; + + /** Gradle property for an exact database-test fork count. */ + public static final String FORKS_PROPERTY = "coreDatabaseForks"; + + /** Environment variable for an exact database-test fork count. */ + public static final String FORKS_ENVIRONMENT_VARIABLE = "CORE_DATABASE_FORKS"; + + /** Root-project extra-property key for the mac-docker-connector fixed-network predicate. */ + public static final String MAC_DOCKER_CONNECTOR_FIXED_NETWORK_EXTRA = + "macDockerConnectorFixedNetwork"; + + /** Maximum safe fork count for a Docker lane using mac-docker-connector's fixed network. */ + public static final int MAC_DOCKER_CONNECTOR_MAX_FORKS = 1; + + /** Indicates that a lane has no task-specific maximum. */ + public static final int NO_LANE_MAXIMUM = 0; + + private CoreDatabaseConcurrency() {} + + /** Identifies where an operator-configurable memory value came from. */ + public enum ConfigurationSource { + /** A Gradle property supplied the value. */ + PROPERTY, + /** An environment variable supplied the value. */ + ENVIRONMENT, + /** The built-in conservative default supplied the value. */ + DEFAULT + } + + /** Identifies how the selected fork count was resolved. */ + public enum Source { + /** The {@link #FORKS_PROPERTY} value was used. */ + PROPERTY, + /** The {@link #FORKS_ENVIRONMENT_VARIABLE} value was used. */ + ENVIRONMENT, + /** All detected machine and build limits were used. */ + DETECTED, + /** Capacity detection was absent or partial, so the serial fallback was used. */ + DEFAULT + } + + /** Resource that constrained the calculated fork count. */ + public enum LimitingFactor { + /** Processor count, divided across the selected database lanes. */ + CPU, + /** Available memory, divided across the selected database lanes. */ + MEMORY, + /** Gradle worker count, divided across the selected database lanes. */ + GRADLE, + /** Operational rollout ceiling. */ + ROLLOUT, + /** Task-specific lane constraint. */ + LANE, + /** Conservative fallback used when capacity detection is absent or partial. */ + DEFAULT + } + + /** + * Resolves the configurable memory assumptions without reading process-global state. + * + *

Each Gradle property takes precedence over its corresponding environment variable. + * Configured values must be positive whole MiB counts. + * + * @param buildReservePropertyValue value of {@link #BUILD_MEMORY_RESERVE_PROPERTY}, or null + * @param buildReserveEnvironmentValue value of {@link + * #BUILD_MEMORY_RESERVE_ENVIRONMENT_VARIABLE}, or null + * @param forkOverheadPropertyValue value of {@link #FORK_MEMORY_OVERHEAD_PROPERTY}, or null + * @param forkOverheadEnvironmentValue value of {@link + * #FORK_MEMORY_OVERHEAD_ENVIRONMENT_VARIABLE}, or null + * @return the resolved memory assumptions and their sources + */ + public static MemoryConfiguration resolveMemoryConfiguration( + @Nullable String buildReservePropertyValue, + @Nullable String buildReserveEnvironmentValue, + @Nullable String forkOverheadPropertyValue, + @Nullable String forkOverheadEnvironmentValue) { + ResolvedBytes buildReserve = + resolvePositiveMebibytes( + buildReservePropertyValue, + buildReserveEnvironmentValue, + DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES, + BUILD_MEMORY_RESERVE_PROPERTY, + BUILD_MEMORY_RESERVE_ENVIRONMENT_VARIABLE); + ResolvedBytes forkOverhead = + resolvePositiveMebibytes( + forkOverheadPropertyValue, + forkOverheadEnvironmentValue, + DEFAULT_TEST_WORKER_OVERHEAD_BYTES, + FORK_MEMORY_OVERHEAD_PROPERTY, + FORK_MEMORY_OVERHEAD_ENVIRONMENT_VARIABLE); + + return new MemoryConfiguration( + buildReserve.bytes(), buildReserve.source(), forkOverhead.bytes(), forkOverhead.source()); + } + + /** + * Returns the conservative per-fork resource budget using resolved memory assumptions. + * + * @param memoryConfiguration resolved build and worker memory assumptions + * @return per-fork memory budget and rollout ceiling + */ + public static Budget rolloutBudget(MemoryConfiguration memoryConfiguration) { + if (memoryConfiguration == null) { + throw new IllegalArgumentException("memory configuration must not be null"); + } + if (memoryConfiguration.buildReserveBytes() <= 0) { + throw new IllegalArgumentException("build reserve bytes must be positive"); + } + if (memoryConfiguration.forkOverheadBytes() <= 0) { + throw new IllegalArgumentException("fork overhead bytes must be positive"); + } + + try { + return new Budget( + Math.addExact(TEST_WORKER_MAX_HEAP_BYTES, memoryConfiguration.forkOverheadBytes()), + memoryConfiguration.buildReserveBytes(), + ROLLOUT_CAP); + } catch (ArithmeticException e) { + throw new IllegalArgumentException( + "test-worker memory budget exceeds the supported range", e); + } + } + + /** + * Calculates memory available to database-test workers from JVM-visible physical memory. + * + * @param totalMemoryBytes total memory visible to the Gradle JVM; zero means unavailable + * @param buildReserveBytes memory retained for Gradle and other build processes + * @return memory left after the build-process reserve + */ + public static long availableMemoryForTestWorkers(long totalMemoryBytes, long buildReserveBytes) { + if (totalMemoryBytes < 0) { + throw new IllegalArgumentException("total memory bytes must not be negative"); + } + if (buildReserveBytes <= 0) { + throw new IllegalArgumentException("build reserve bytes must be positive"); + } + if (totalMemoryBytes == 0) { + return 0; + } + return Math.max(0, totalMemoryBytes - buildReserveBytes); + } + + /** + * Returns the fixed lane maximum for a Docker task using mac-docker-connector. + * + * @param dockerLane whether the database lane requires Docker + * @param macDockerConnectorFixedNetwork whether the shared fixed network is active + * @return one when both inputs are true, otherwise {@link #NO_LANE_MAXIMUM} + */ + public static int laneMaximum(boolean dockerLane, boolean macDockerConnectorFixedNetwork) { + return dockerLane && macDockerConnectorFixedNetwork + ? MAC_DOCKER_CONNECTOR_MAX_FORKS + : NO_LANE_MAXIMUM; + } + + /** + * Resolves the fork count without reading process-global state. + * + *

The Gradle property takes precedence over the environment variable. Exact requests are + * checked against every known capacity bound, the rollout cap, and the lane maximum. Without an + * exact request, every capacity signal must be known before automatic parallelism is enabled. + * Machine capacity is divided across the database lanes selected by the same Gradle invocation. + * + * @param propertyValue value of {@link #FORKS_PROPERTY}, or null + * @param environmentValue value of {@link #FORKS_ENVIRONMENT_VARIABLE}, or null + * @param capacity detected machine, build, and lane capacity + * @param budget per-fork memory budget and rollout cap + * @param laneMaximum task-specific maximum, or {@link #NO_LANE_MAXIMUM} + * @return the selected fork count and its resolution details + */ + public static Resolution resolve( + @Nullable String propertyValue, + @Nullable String environmentValue, + DetectedCapacity capacity, + Budget budget, + int laneMaximum) { + validate(capacity, budget, laneMaximum); + + CapacityLimit capacityLimit = calculateCapacityLimit(capacity, budget); + AppliedLimit allowedLimit = calculateAllowedLimit(capacityLimit, budget, laneMaximum); + String exactValue = propertyValue != null ? propertyValue : environmentValue; + Source exactSource = propertyValue != null ? Source.PROPERTY : Source.ENVIRONMENT; + + if (exactValue != null) { + int requestedForks = parseExactForks(exactValue, exactSource); + if (requestedForks > allowedLimit.maximum()) { + throw new IllegalArgumentException( + String.format( + "%s requests %d database forks, but the allowed maximum is %d " + + "(known per-lane capacity maximum %s, rollout cap %d, lane maximum %s)", + sourceName(exactSource), + requestedForks, + allowedLimit.maximum(), + displayMaximum(capacityLimit.detected(), capacityLimit.maximum()), + budget.rolloutCap(), + displayMaximum(laneMaximum > 0, laneMaximum))); + } + return new Resolution( + requestedForks, + capacityLimit.detected() ? capacityLimit.maximum() : 0, + allowedLimit.maximum(), + exactSource, + allowedLimit.factor()); + } + + if (!capacityLimit.fullyDetected()) { + return new Resolution( + DEFAULT_FORKS, + capacityLimit.detected() ? capacityLimit.maximum() : 0, + allowedLimit.maximum(), + Source.DEFAULT, + LimitingFactor.DEFAULT); + } + + return new Resolution( + allowedLimit.maximum(), + capacityLimit.maximum(), + allowedLimit.maximum(), + Source.DETECTED, + allowedLimit.factor()); + } + + private static void validate(DetectedCapacity capacity, Budget budget, int laneMaximum) { + if (capacity == null) { + throw new IllegalArgumentException("detected capacity must not be null"); + } + if (budget == null) { + throw new IllegalArgumentException("resource budget must not be null"); + } + if (capacity.processors() < 0) { + throw new IllegalArgumentException("detected processor count must not be negative"); + } + if (capacity.totalMemoryBytes() < 0) { + throw new IllegalArgumentException("detected total memory must not be negative"); + } + if (capacity.gradleMaxWorkers() < 0) { + throw new IllegalArgumentException("Gradle maximum workers must not be negative"); + } + if (capacity.activeDatabaseLanes() <= 0) { + throw new IllegalArgumentException("active database lane count must be positive"); + } + if (budget.memoryBytesPerFork() <= 0) { + throw new IllegalArgumentException("memory bytes per fork must be positive"); + } + if (budget.buildReserveBytes() <= 0) { + throw new IllegalArgumentException("build reserve bytes must be positive"); + } + if (budget.rolloutCap() <= 0) { + throw new IllegalArgumentException("rollout cap must be positive"); + } + if (laneMaximum < 0) { + throw new IllegalArgumentException("lane maximum must not be negative"); + } + } + + private static CapacityLimit calculateCapacityLimit(DetectedCapacity capacity, Budget budget) { + int maximum = Integer.MAX_VALUE; + LimitingFactor factor = LimitingFactor.DEFAULT; + int detectedSignals = 0; + + if (capacity.processors() > 0) { + maximum = perLane(capacity.processors(), capacity.activeDatabaseLanes()); + factor = LimitingFactor.CPU; + detectedSignals++; + } + + if (capacity.totalMemoryBytes() > 0) { + long availableMemoryBytes = + availableMemoryForTestWorkers(capacity.totalMemoryBytes(), budget.buildReserveBytes()); + long memoryForks = + Math.max(DEFAULT_FORKS, availableMemoryBytes / budget.memoryBytesPerFork()); + int boundedMemoryForks = (int) Math.min(memoryForks, Integer.MAX_VALUE); + int memoryForksPerLane = perLane(boundedMemoryForks, capacity.activeDatabaseLanes()); + if (memoryForksPerLane < maximum) { + maximum = memoryForksPerLane; + factor = LimitingFactor.MEMORY; + } + detectedSignals++; + } + + if (capacity.gradleMaxWorkers() > 0) { + int gradleWorkersPerLane = + perLane(capacity.gradleMaxWorkers(), capacity.activeDatabaseLanes()); + if (gradleWorkersPerLane < maximum) { + maximum = gradleWorkersPerLane; + factor = LimitingFactor.GRADLE; + } + detectedSignals++; + } + + if (detectedSignals == 0) { + return new CapacityLimit(0, LimitingFactor.DEFAULT, false, false); + } + return new CapacityLimit(maximum, factor, true, detectedSignals == 3); + } + + private static AppliedLimit calculateAllowedLimit( + CapacityLimit capacityLimit, Budget budget, int laneMaximum) { + int maximum = budget.rolloutCap(); + LimitingFactor factor = LimitingFactor.ROLLOUT; + + if (capacityLimit.detected() && capacityLimit.maximum() <= maximum) { + maximum = capacityLimit.maximum(); + factor = capacityLimit.factor(); + } + if (laneMaximum > 0 && laneMaximum <= maximum) { + maximum = laneMaximum; + factor = LimitingFactor.LANE; + } + + return new AppliedLimit(maximum, factor); + } + + private static int perLane(int capacity, int activeDatabaseLanes) { + return Math.max(DEFAULT_FORKS, capacity / activeDatabaseLanes); + } + + private static ResolvedBytes resolvePositiveMebibytes( + @Nullable String propertyValue, + @Nullable String environmentValue, + long defaultBytes, + String propertyName, + String environmentName) { + if (propertyValue != null) { + return new ResolvedBytes( + parsePositiveMebibytes(propertyValue, propertyName), ConfigurationSource.PROPERTY); + } + if (environmentValue != null) { + return new ResolvedBytes( + parsePositiveMebibytes(environmentValue, environmentName), + ConfigurationSource.ENVIRONMENT); + } + return new ResolvedBytes(defaultBytes, ConfigurationSource.DEFAULT); + } + + private static long parsePositiveMebibytes(String value, String name) { + String trimmed = value.trim(); + if (trimmed.isEmpty()) { + throw positiveMebibytesError(name, null); + } + + try { + long mebibytes = Long.parseLong(trimmed); + if (mebibytes <= 0) { + throw positiveMebibytesError(name, null); + } + return Math.multiplyExact(mebibytes, MIB); + } catch (NumberFormatException | ArithmeticException e) { + throw positiveMebibytesError(name, e); + } + } + + private static IllegalArgumentException positiveMebibytesError( + String name, @Nullable RuntimeException cause) { + String message = name + " must be a positive whole number of MiB"; + return cause == null + ? new IllegalArgumentException(message) + : new IllegalArgumentException(message, cause); + } + + private static int parseExactForks(String value, Source source) { + String trimmed = value.trim(); + if (trimmed.isEmpty()) { + throw new IllegalArgumentException(sourceName(source) + " must be a positive integer"); + } + + try { + int parsed = Integer.parseInt(trimmed); + if (parsed <= 0) { + throw new IllegalArgumentException(sourceName(source) + " must be a positive integer"); + } + return parsed; + } catch (NumberFormatException e) { + throw new IllegalArgumentException(sourceName(source) + " must be a positive integer", e); + } + } + + private static String sourceName(Source source) { + return source == Source.PROPERTY ? FORKS_PROPERTY : FORKS_ENVIRONMENT_VARIABLE; + } + + private static String displayMaximum(boolean known, int maximum) { + return known ? Integer.toString(maximum) : "unknown"; + } + + /** + * Resolved build-process and per-worker memory assumptions. + * + * @param buildReserveBytes memory retained for Gradle and other build processes + * @param buildReserveSource input that supplied {@code buildReserveBytes} + * @param forkOverheadBytes native and database-container allowance per fork + * @param forkOverheadSource input that supplied {@code forkOverheadBytes} + */ + public record MemoryConfiguration( + long buildReserveBytes, + ConfigurationSource buildReserveSource, + long forkOverheadBytes, + ConfigurationSource forkOverheadSource) {} + + /** + * Machine, Gradle, and lane capacity detected by the caller. + * + *

A zero processor, memory, or Gradle-worker value means that signal was unavailable. + * + * @param processors available processors + * @param totalMemoryBytes total memory visible to the Gradle JVM before the build reserve + * @param gradleMaxWorkers maximum Gradle workers available to the build + * @param activeDatabaseLanes database lanes selected by the same Gradle invocation + */ + public record DetectedCapacity( + int processors, long totalMemoryBytes, int gradleMaxWorkers, int activeDatabaseLanes) {} + + /** + * Resource budget for a database-test fork. + * + * @param memoryBytesPerFork memory reserved for each fork + * @param buildReserveBytes memory retained for Gradle and other build processes + * @param rolloutCap operational ceiling for parallel forks + */ + public record Budget(long memoryBytesPerFork, long buildReserveBytes, int rolloutCap) {} + + /** + * Result of resolving database-test concurrency. + * + * @param forks selected fork count + * @param safeMaximum smallest known per-lane capacity bound, or zero when none is known + * @param allowedMaximum smallest detected, rollout, and task-specific lane bound + * @param source input that selected {@code forks} + * @param limitingFactor bound that determined {@code allowedMaximum} + */ + public record Resolution( + int forks, + int safeMaximum, + int allowedMaximum, + Source source, + LimitingFactor limitingFactor) {} + + private record ResolvedBytes(long bytes, ConfigurationSource source) {} + + private record CapacityLimit( + int maximum, LimitingFactor factor, boolean detected, boolean fullyDetected) {} + + private record AppliedLimit(int maximum, LimitingFactor factor) {} +} diff --git a/buildSrc/src/main/java/org/apache/gravitino/testing/DbConnectionInfo.java b/buildSrc/src/main/java/org/apache/gravitino/testing/DbConnectionInfo.java new file mode 100644 index 00000000000..2d2fc3b859c --- /dev/null +++ b/buildSrc/src/main/java/org/apache/gravitino/testing/DbConnectionInfo.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +/** + * Administrative connection info for a {@link SharedDbContainerService}-managed shared database + * container, not naming any particular database on that server. + */ +public final class DbConnectionInfo { + + private final String adminJdbcUrl; + private final String user; + private final String password; + + /** + * Creates a new connection info holder. + * + * @param adminJdbcUrl a JDBC URL that can be connected to without naming any database + * @param user the administrative user + * @param password the password for {@code user} + */ + public DbConnectionInfo(String adminJdbcUrl, String user, String password) { + this.adminJdbcUrl = adminJdbcUrl; + this.user = user; + this.password = password; + } + + /** Returns the administrative JDBC URL, not naming any database. */ + public String getAdminJdbcUrl() { + return adminJdbcUrl; + } + + /** Returns the administrative user. */ + public String getUser() { + return user; + } + + /** Returns the password for {@link #getUser()}. */ + public String getPassword() { + return password; + } +} diff --git a/buildSrc/src/main/java/org/apache/gravitino/testing/SharedDbContainerService.java b/buildSrc/src/main/java/org/apache/gravitino/testing/SharedDbContainerService.java new file mode 100644 index 00000000000..9f4ba68a513 --- /dev/null +++ b/buildSrc/src/main/java/org/apache/gravitino/testing/SharedDbContainerService.java @@ -0,0 +1,633 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; +import org.gradle.api.services.BuildService; +import org.gradle.api.services.BuildServiceParameters; + +/** + * A Gradle {@link BuildService} that owns a single, shared MySQL container and a single, shared + * PostgreSQL container for the whole lifetime of the build, so that the Core database test tasks + * no longer have to start one container per Gradle test-worker fork. + * + *

Containers are started lazily, on the first call to {@link #connectionInfo(String)} for a + * given backend, and are reused for every subsequent call for that backend within the same build. + * Each forked test JVM is expected to create its own database on the shared container rather than + * requesting its own container. + * + *

The main Gradle process does not have Testcontainers on its classpath, so containers are + * managed directly through the {@code docker} CLI rather than through Testcontainers. Every + * container this service starts is tagged with the {@value #CONTAINER_LABEL_KEY} label plus a + * {@value #CONTAINER_PID_LABEL_KEY} label recording the Gradle daemon process that started it, so + * that a crashed or forcibly killed build does not leak a container forever: the next build that + * creates this service removes any labelled container that has either exited, or is still + * running but whose recording daemon process is no longer alive - a still-running container whose + * daemon *is* alive belongs to a build genuinely still in progress (a second Gradle invocation, + * another CI job sharing this runner, an IDE test run started mid-build) and is left alone. + * Databases created on a container are never individually dropped; they are discarded together + * with the container when it is removed. + */ +public abstract class SharedDbContainerService + implements BuildService, AutoCloseable { + + private static final Logger LOG = Logging.getLogger(SharedDbContainerService.class); + + /** Docker label key applied to every container this service starts. */ + private static final String CONTAINER_LABEL_KEY = "gravitino-core-test"; + + /** + * Docker label key recording the PID of the Gradle daemon process that started a container, so + * a later build can tell "still running because its build is still in progress" apart from + * "still running because its build (and daemon) died without cleaning up". + */ + private static final String CONTAINER_PID_LABEL_KEY = "gravitino-core-test-pid"; + + private static final String MYSQL_BACKEND = "mysql"; + private static final String POSTGRESQL_BACKEND = "postgresql"; + + private static final String MYSQL_IMAGE = "mysql:8.0"; + private static final String POSTGRESQL_IMAGE = "postgres:13"; + + private static final String MYSQL_CONTAINER_PORT = "3306"; + private static final String POSTGRESQL_CONTAINER_PORT = "5432"; + + private static final String DB_USER = "root"; + private static final String DB_PASSWORD = "root"; + + private static final int READINESS_MAX_ATTEMPTS = 120; + private static final long READINESS_POLL_INTERVAL_MILLIS = 1000L; + + // How long a single docker CLI invocation (including a cold image pull) may run before it is + // treated as hung and killed. + /** Number of trailing container-log lines echoed into the readiness-timeout exception. */ + private static final int CONTAINER_LOG_TAIL_LINES = 20; + + private static final long EXEC_TIMEOUT_SECONDS = 300L; + + // Unique per build-service instance, embedded in every container this instance starts (see + // startMySql/startPostgreSql). Purely informational/diagnostic - see CONTAINER_PID_LABEL_KEY + // for the label removeStaleContainers() actually uses to decide what is safe to remove. + private final String labelValue = "run-" + UUID.randomUUID(); + + // The current Gradle daemon's PID, recorded on every container this instance starts (see + // CONTAINER_PID_LABEL_KEY / removeStaleContainers()). + private final long ownerPid = ProcessHandle.current().pid(); + + private final Map connectionInfoByBackend = new ConcurrentHashMap<>(); + private final Map containerIdByBackend = new ConcurrentHashMap<>(); + + // Drains a docker CLI child process's stdout/stderr concurrently (see exec()). Daemon threads + // so a stuck drain never keeps the Gradle daemon process alive. + private final ExecutorService streamReader = + Executors.newCachedThreadPool( + runnable -> { + Thread thread = new Thread(runnable, "shared-db-container-stream-reader"); + thread.setDaemon(true); + return thread; + }); + + /** Creates the service and removes any stale containers left by a previously killed build. */ + public SharedDbContainerService() { + removeStaleContainers(); + } + + /** + * Returns connection info for the shared container for the given backend, starting that + * backend's container on first use. + * + * @param backend either {@code "mysql"} or {@code "postgresql"} + * @return administrative connection info for the shared container, not naming any database + */ + public synchronized DbConnectionInfo connectionInfo(String backend) { + return connectionInfoByBackend.computeIfAbsent(backend, this::startContainer); + } + + /** Stops every container this service instance started. */ + @Override + public void close() { + containerIdByBackend.values().forEach(this::removeContainer); + containerIdByBackend.clear(); + streamReader.shutdownNow(); + } + + private void removeContainer(String containerId) { + try { + runDocker("rm", "-f", containerId); + } catch (RuntimeException e) { + LOG.warn("Failed to stop shared test database container {}: {}", containerId, e.getMessage()); + } + } + + /** + * Removes containers left behind by a previous, no-longer-running build. + * + *

A labelled container with {@code status=exited} is always reaped (its owning build is, by + * definition, done with it). A labelled container that is still {@code running} is reaped only + * when its recorded {@value #CONTAINER_PID_LABEL_KEY} process is no longer alive - that is the + * one realistic leak this class exists to close: a Gradle daemon that was SIGKILLed, OOM-killed, + * or lost with its host, which never got to call {@link #close()}. A still-running container + * whose daemon process *is* alive is never touched, since it belongs to a build genuinely still + * in progress on this host (a second Gradle invocation, another CI job sharing this runner, an + * IDE test run started mid-build). + */ + private void removeStaleContainers() { + String labelledIds = + runDockerQuiet("ps", "-aq", "--filter", "label=" + CONTAINER_LABEL_KEY); + List ids = splitNonBlankLines(labelledIds); + List toRemove = new ArrayList<>(); + for (String id : ids) { + if (isReapable(id)) { + toRemove.add(id); + } + } + if (toRemove.isEmpty()) { + return; + } + + LOG.lifecycle( + "Removing {} stale shared test database container(s) from a previous build", toRemove.size()); + List removeCommand = new ArrayList<>(); + removeCommand.add("rm"); + removeCommand.add("-f"); + removeCommand.addAll(toRemove); + runDockerQuiet(removeCommand.toArray(new String[0])); + } + + /** + * Returns whether the given labelled container may be safely removed: it has already exited, + * or its recorded owning daemon process is no longer alive. + */ + private boolean isReapable(String containerId) { + String state = + runDockerQuiet("inspect", "-f", "{{.State.Status}}", containerId).trim(); + if ("exited".equals(state)) { + return true; + } + + String pidLabel = + runDockerQuiet( + "inspect", + "-f", + "{{index .Config.Labels \"" + CONTAINER_PID_LABEL_KEY + "\"}}", + containerId) + .trim(); + return !isOwningProcessAlive(pidLabel); + } + + /** Returns whether {@code pidLabel} parses to a PID that is currently alive on this host. */ + static boolean isOwningProcessAlive(String pidLabel) { + try { + long pid = Long.parseLong(pidLabel.trim()); + // PIDs 0 and 1 are the kernel/init on macOS and Linux, never a Gradle daemon. + return pid > 1 && ProcessHandle.of(pid).map(ProcessHandle::isAlive).orElse(false); + } catch (NumberFormatException e) { + // No usable PID label (e.g. a container from before this label existed, or empty output) - + // treat it as not verifiably alive, so removeStaleContainers() falls through to reaping it + // rather than leaking it forever on a false-negative liveness check. + return false; + } + } + + + /** + * Why this tuning exists at all: one shared container serves roughly twice the concurrent + * DDL/DML load that per-fork containers each saw, because every test fork now writes to the + * same server. A database's production durability defaults -- an fsync on every commit, frequent + * checkpoints, doublewrite/full-page writes, binary logging -- are per-commit costs, so that + * doubled write load multiplies them rather than absorbing them. Relaxing those guarantees for + * a container that is disposable and torn down at the end of the run was the difference between + * the shared container being a ~5.6% regression and being a 3x+ speedup. + * + *

Loads extra container-start args for {@code backend} from a plain-text resource at + * {@code shared-db-tuning/.args} (one token per line; blank lines and lines + * starting with {@code #} are ignored). Missing resources or read failures are logged and + * treated as "no extra args" so a bad/missing tuning file can never fail the build -- + * it only ever falls back to untuned defaults. + */ + private List loadExtraDbArgs(String backend) { + // Resolved relative to this class's own package via Class#getResourceAsStream, NOT + // ClassLoader#getResourceAsStream: the loader variant treats the name as absolute from the + // classpath root, so "shared-db-tuning/mysql.args" never matched the file's actual location + // (org/apache/gravitino/testing/shared-db-tuning/mysql.args) and every lookup silently + // returned null, leaving both containers on stock server defaults. + String resourcePath = "shared-db-tuning/" + backend + ".args"; + try (java.io.InputStream in = + SharedDbContainerService.class.getResourceAsStream(resourcePath)) { + if (in == null) { + LOG.warn( + "No DB tuning args found on the classpath at {} for backend {}; the shared container" + + " will run with untuned server defaults", + resourcePath, + backend); + return java.util.Collections.emptyList(); + } + List args = new ArrayList<>(); + try (java.io.BufferedReader reader = + new java.io.BufferedReader(new java.io.InputStreamReader(in, StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + String trimmed = line.trim(); + if (!trimmed.isEmpty() && !trimmed.startsWith("#")) { + args.add(trimmed); + } + } + } + return args; + } catch (IOException e) { + LOG.warn("Failed to load DB tuning args for {} from {}, continuing untuned", + backend, resourcePath, e); + return java.util.Collections.emptyList(); + } + } + + /** + * Logs the tuning args actually applied to a shared container, so a run that is unexpectedly + * slow can be told apart from a run whose tuning file silently failed to load. + */ + private void logTuningArgs(String backendLabel, List tuningArgs) { + if (tuningArgs.isEmpty()) { + LOG.warn( + "Starting shared {} test database container with NO tuning args (untuned server" + + " defaults); expect a substantially slower run", + backendLabel); + } else { + LOG.lifecycle( + "Applying {} tuning args to shared {} test database container: {}", + tuningArgs.size(), + backendLabel, + String.join(" ", tuningArgs)); + } + } + + private DbConnectionInfo startContainer(String backend) { + switch (backend) { + case MYSQL_BACKEND: + return startMySql(); + case POSTGRESQL_BACKEND: + return startPostgreSql(); + default: + throw new IllegalArgumentException("Unsupported shared test database backend: " + backend); + } + } + + private DbConnectionInfo startMySql() { + List mysqlRunArgs = + new ArrayList<>( + Arrays.asList( + "run", + "-d", + "--rm", + "--label", + CONTAINER_LABEL_KEY + "=" + labelValue, + "--label", + CONTAINER_PID_LABEL_KEY + "=" + ownerPid, + "-P", + // Disposable container -- backing the data dir with tmpfs removes real disk + // I/O for every write, on top of the durability-relaxing mysqld flags below. + // + // The size cap is mandatory, not cosmetic: an uncapped `--tmpfs` mount defaults + // to 50% of host RAM *per mount*, so MySQL's and PostgreSQL's data dirs could + // between them claim 100% of RAM and get the Gradle daemon or a test fork + // OOM-killed. Headroom math for this repo's real lane -- the backend + // integration-test job in .github/workflows/backend-integration-test.yml runs + // on `ubuntu-22.04`, a GitHub-hosted runner with 4 vCPU / 16 GiB RAM: + // 16 GiB total + // - 4 GiB build-process reserve (CoreDatabaseConcurrency + // DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES) + // - 12 GiB 2 concurrent test forks x (4 GiB max heap, + // TEST_WORKER_MAX_HEAP_MIB + 2 GiB native overhead, + // DEFAULT_TEST_WORKER_OVERHEAD_BYTES) + // i.e. the JVM side alone nominally commits the whole box, so these mounts must + // stay a bounded worst case rather than an open-ended 8 GiB each. MySQL's share + // is 2g: the core test schema is small, and --skip-log-bin plus the default + // ~100 MiB redo capacity mean little but table data lands here, while + // --innodb-buffer-pool-size=2G is separately resident in the container's RSS. + "--tmpfs", + "/var/lib/mysql:size=2g", + "-e", + "MYSQL_ROOT_PASSWORD=" + DB_PASSWORD, + MYSQL_IMAGE)); + List mysqlTuningArgs = loadExtraDbArgs("mysql"); + logTuningArgs("MySQL", mysqlTuningArgs); + mysqlRunArgs.addAll(mysqlTuningArgs); + String containerId = runDocker(mysqlRunArgs.toArray(new String[0])); + try { + awaitReady( + containerId, + new String[] { + "mysqladmin", "ping", "-h", "127.0.0.1", "-u" + DB_USER, "-p" + DB_PASSWORD + }, + "MySQL"); + int hostPort = resolveHostPort(containerId, MYSQL_CONTAINER_PORT); + containerIdByBackend.put(MYSQL_BACKEND, containerId); + + String adminJdbcUrl = String.format("jdbc:mysql://127.0.0.1:%d", hostPort); + LOG.lifecycle( + "Started shared MySQL test database container {} on port {}", containerId, hostPort); + return new DbConnectionInfo(adminJdbcUrl, DB_USER, DB_PASSWORD); + } catch (RuntimeException e) { + // Not yet recorded in containerIdByBackend (only done on success above), so close() would + // never reap this container - remove it now instead of leaking it until the next build's + // stale-container cleanup. + removeContainer(containerId); + throw e; + } + } + + private DbConnectionInfo startPostgreSql() { + List postgresRunArgs = + new ArrayList<>( + Arrays.asList( + "run", + "-d", + "--rm", + "--label", + CONTAINER_LABEL_KEY + "=" + labelValue, + "--label", + CONTAINER_PID_LABEL_KEY + "=" + ownerPid, + "-P", + // Disposable container -- see the equivalent tmpfs mount in startMySql() for + // the runner spec (ubuntu-22.04, 4 vCPU / 16 GiB) and the headroom math. + // + // PostgreSQL gets a larger cap than MySQL (5g vs 2g) because pg_wal lives + // *inside* the data dir and postgresql.args sets max_wal_size=4GB with + // checkpoint_timeout=1h, so up to ~4 GiB of WAL can legitimately accumulate + // here before a checkpoint recycles it. Capping this at MySQL's 2g would turn a + // merely-late checkpoint into an ENOSPC and a PostgreSQL PANIC partway through + // the run. 5g = ~4 GiB WAL headroom + ~1 GiB for the schema and its data. + // Worst case both mounts together top out at 7 GiB, leaving ~9 GiB for the OS, + // the two database processes' RSS (2 GiB buffer pool + 1 GiB shared_buffers, + // plus overhead) and the fork JVMs' actual (as opposed to maximum) heap usage. + "--tmpfs", + "/var/lib/postgresql/data:size=5g", + "-e", + "POSTGRES_USER=" + DB_USER, + "-e", + "POSTGRES_PASSWORD=" + DB_PASSWORD, + POSTGRESQL_IMAGE)); + List postgresTuningArgs = loadExtraDbArgs("postgresql"); + logTuningArgs("PostgreSQL", postgresTuningArgs); + postgresRunArgs.addAll(postgresTuningArgs); + String containerId = runDocker(postgresRunArgs.toArray(new String[0])); + try { + // -h forces a TCP health check. The postgres image's entrypoint first runs a *temporary*, + // socket-only server (listen_addresses='') to execute init scripts before starting the + // real, TCP-listening server; a socket-based pg_isready (no -h) reports ready during that + // window, before this container can actually accept the JDBC connection made right after. + awaitReady(containerId, new String[] {"pg_isready", "-h", "127.0.0.1", "-U", DB_USER}, "PostgreSQL"); + int hostPort = resolveHostPort(containerId, POSTGRESQL_CONTAINER_PORT); + containerIdByBackend.put(POSTGRESQL_BACKEND, containerId); + + // Mirrors PostgreSQLContainer#getJdbcUrl(): no database name, trailing slash. + String adminJdbcUrl = String.format("jdbc:postgresql://127.0.0.1:%d/", hostPort); + LOG.lifecycle( + "Started shared PostgreSQL test database container {} on port {}", + containerId, + hostPort); + return new DbConnectionInfo(adminJdbcUrl, DB_USER, DB_PASSWORD); + } catch (RuntimeException e) { + removeContainer(containerId); + throw e; + } + } + + private int resolveHostPort(String containerId, String containerPort) { + String output = runDocker("port", containerId, containerPort + "/tcp"); + return parseHostPort(output, containerId); + } + + /** + * Parses the host port `docker port <container> <port>/tcp` publishes from its raw + * output, e.g. {@code "0.0.0.0:49153"} or, when the daemon also publishes an IPv6 address, + * {@code "0.0.0.0:49153\n[::]:49153"} (the first line is used either way). + * + * @throws IllegalStateException if {@code output} has no lines (no published port) + * @throws NumberFormatException if the port segment after the last {@code ':'} is not numeric + */ + static int parseHostPort(String output, String containerId) { + List lines = splitNonBlankLines(output); + if (lines.isEmpty()) { + throw new IllegalStateException( + "docker port did not report a published host port for container " + containerId); + } + String firstLine = lines.get(0); + String portPart = firstLine.substring(firstLine.lastIndexOf(':') + 1).trim(); + return Integer.parseInt(portPart); + } + + private void awaitReady(String containerId, String[] healthCheckCommand, String backendLabel) { + List execCommand = new ArrayList<>(); + execCommand.add("exec"); + execCommand.add(containerId); + execCommand.addAll(Arrays.asList(healthCheckCommand)); + + for (int attempt = 1; attempt <= READINESS_MAX_ATTEMPTS; attempt++) { + if (runDockerExitCode(execCommand) == 0) { + return; + } + sleep(READINESS_POLL_INTERVAL_MILLIS); + } + + // Capture the container's own logs before returning: the caller removes a container that + // failed to become ready, which destroys the only evidence of *why* it failed (bad server + // flag, ENOSPC on the tmpfs data dir, image init script error). Done here rather than in the + // caller so it runs before any removal path. + String containerLogs = captureContainerLogs(containerId); + LOG.error( + "Shared {} test database container {} never became ready. Full container logs follow:\n{}", + backendLabel, + containerId, + containerLogs); + + throw new IllegalStateException( + String.format( + "Timed out after %d attempts waiting for shared %s test database container %s to" + + " become ready. Last %d lines of container logs:%n%s", + READINESS_MAX_ATTEMPTS, + backendLabel, + containerId, + CONTAINER_LOG_TAIL_LINES, + tailLines(containerLogs, CONTAINER_LOG_TAIL_LINES))); + } + + /** + * Returns the combined stdout and stderr of {@code docker logs }, or a short + * placeholder if the logs cannot be read. Never throws: this only ever runs on a path that is + * already failing, and losing the diagnostic must not mask the original error. Both streams are + * collected because the MySQL and PostgreSQL images write their server logs to stderr. + */ + private String captureContainerLogs(String containerId) { + try { + ProcessResult result = exec("logs", containerId); + StringBuilder combined = new StringBuilder(); + if (!result.stdout.isEmpty()) { + combined.append(result.stdout); + } + if (!result.stderr.isEmpty()) { + if (combined.length() > 0) { + combined.append(System.lineSeparator()); + } + combined.append(result.stderr); + } + return combined.length() == 0 ? "" : combined.toString(); + } catch (RuntimeException e) { + LOG.warn("Failed to capture logs for container {}", containerId, e); + return ""; + } + } + + /** Returns at most the last {@code maxLines} non-blank lines of {@code text}. */ + static String tailLines(String text, int maxLines) { + List lines = splitNonBlankLines(text); + List tail = lines.subList(Math.max(0, lines.size() - maxLines), lines.size()); + return String.join(System.lineSeparator(), tail); + } + + private void sleep(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while waiting for shared test database container", e); + } + } + + /** Runs {@code docker }, throwing if it exits non-zero, and returns its stdout. */ + private String runDocker(String... args) { + ProcessResult result = exec(args); + if (result.exitCode != 0) { + throw new IllegalStateException( + "docker " + String.join(" ", args) + " failed with exit code " + result.exitCode + + ": " + result.stderr); + } + return result.stdout; + } + + /** Runs {@code docker }, logging (but not throwing on) a non-zero exit, and returns stdout. */ + private String runDockerQuiet(String... args) { + ProcessResult result = exec(args); + if (result.exitCode != 0) { + LOG.warn("docker {} exited with {}: {}", String.join(" ", args), result.exitCode, result.stderr); + } + return result.stdout; + } + + /** Runs {@code docker } and returns only its exit code. */ + private int runDockerExitCode(List args) { + return exec(args.toArray(new String[0])).exitCode; + } + + private ProcessResult exec(String... args) { + List command = new ArrayList<>(); + command.add("docker"); + command.addAll(Arrays.asList(args)); + + Process process = null; + try { + // Deliberately does NOT merge stderr into stdout: `docker run` writes image-pull progress + // to stderr while it is still starting the container, and prints only the container ID to + // stdout on success. Merging the two streams (as redirectErrorStream(true) would) corrupts + // the container ID with pull-progress text on a cold cache, breaking every later `docker + // exec`/`docker port` call keyed on that ID. + // + // stdout and stderr are drained on separate threads, concurrently with the process running: + // a slow image pull can write more to stderr than one pipe buffer holds, and letting stdout + // fill up while stderr is not yet being drained would deadlock against the child process + // blocking on a full stderr pipe. + process = new ProcessBuilder(command).start(); + process.getOutputStream().close(); // no docker subcommand used here reads stdin + Process startedProcess = process; + Future stdoutFuture = + streamReader.submit(() -> startedProcess.getInputStream().readAllBytes()); + Future stderrFuture = + streamReader.submit(() -> startedProcess.getErrorStream().readAllBytes()); + + // waitFor(timeout) is checked BEFORE reading the futures: readAllBytes() only returns at + // EOF (i.e. once the process has actually exited), so blocking on the futures first would + // make this timeout unreachable - a hung `docker` call would block the build forever. + boolean finished = process.waitFor(EXEC_TIMEOUT_SECONDS, TimeUnit.SECONDS); + if (!finished) { + process.destroyForcibly(); + stdoutFuture.cancel(true); + stderrFuture.cancel(true); + throw new IllegalStateException("Command timed out: " + String.join(" ", command)); + } + + String stdout = + new String(stdoutFuture.get(EXEC_TIMEOUT_SECONDS, TimeUnit.SECONDS), StandardCharsets.UTF_8); + String stderr = + new String(stderrFuture.get(EXEC_TIMEOUT_SECONDS, TimeUnit.SECONDS), StandardCharsets.UTF_8); + return new ProcessResult(process.exitValue(), stdout.trim(), stderr.trim()); + } catch (IOException e) { + throw new IllegalStateException("Failed to run command: " + String.join(" ", command), e); + } catch (ExecutionException e) { + throw new IllegalStateException( + "Failed to read output of command: " + String.join(" ", command), e.getCause()); + } catch (TimeoutException e) { + throw new IllegalStateException( + "Timed out reading output of command: " + String.join(" ", command), e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("Interrupted while running command: " + String.join(" ", command), e); + } finally { + if (process != null && process.isAlive()) { + process.destroyForcibly(); + } + } + } + + static List splitNonBlankLines(String text) { + List lines = new ArrayList<>(); + for (String line : text.split("\\R")) { + if (!line.trim().isEmpty()) { + lines.add(line.trim()); + } + } + return lines; + } + + private static final class ProcessResult { + private final int exitCode; + private final String stdout; + private final String stderr; + + private ProcessResult(int exitCode, String stdout, String stderr) { + this.exitCode = exitCode; + this.stdout = stdout; + this.stderr = stderr; + } + } +} diff --git a/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/README.md b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/README.md new file mode 100644 index 00000000000..40961ff0b88 --- /dev/null +++ b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/README.md @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +# Shared test database tuning + +`mysql.args` and `postgresql.args` hold extra server flags that +`SharedDbContainerService` passes to the single shared MySQL / PostgreSQL +container used by the `core` database tests. This file explains what the tuning +does, what it bought, and -- most importantly -- how to re-derive the memory +caps if the CI runner class ever changes. + +## What the tuning does and why + +The `core` DB tests used to get one container per test fork. They now share one +container per backend, which removes a lot of container startup cost but means +a single server absorbs roughly **twice the concurrent DDL/DML** it saw before. + +That matters because a database's default durability guarantees are *per-commit* +costs: an fsync on every commit, doublewrite / full-page writes, binary logging, +and frequent checkpoints. Doubling the write load multiplies those costs rather +than absorbing them, so the shared container started out *slower* than the +per-fork design it replaced. + +The `.args` files relax exactly those guarantees (`fsync=off`, +`innodb_flush_log_at_trx_commit=0`, `synchronous_commit=off`, +`full_page_writes=off`, `--skip-log-bin`, `autovacuum=off`, a long +`checkpoint_timeout`, and a larger buffer pool / `shared_buffers`), and the +service mounts each data dir on `tmpfs`. This is safe **here and only here**: +the container is disposable and is destroyed at the end of the run, so crash +recovery, replication and long-term maintenance have no one to serve. None of +these settings would be acceptable in production. + +One flag is deliberately *not* set: `--performance-schema=OFF`. +`TestRoleMembershipWrites` and `TestOwnerAssignmentWrites` verify real row-lock +contention by polling `performance_schema.data_lock_waits`. With the +performance schema disabled that table is silently always empty, so those +assertions would stop verifying anything without ever failing. + +## Measured effect + +Measured on `apache/gravitino#13553` with `-PcoreDatabaseForks=2`: + +| Task | Before (per-fork) | After (shared + tuned) | Speedup | +| ------------------------- | ----------------- | ---------------------- | ------- | +| `core:coreMySQLTest` | 26m25s | 7m04s | 3.74x | +| `core:corePostgreSQLTest` | 7m10s | 2m11s | 3.28x | +| `core:test (H2)` | 1m27s | 1m21s | ~1.0x (control) | + +The H2 run (`:core:test`) uses no container and is flat across every run, which is what +confirms the speedup is database I/O rather than a faster host. + +### Attribution: the tmpfs data dir did most of the work + +The numbers above were originally collected while a classpath-path bug meant +the `.args` files were never actually loaded (`ClassLoader#getResourceAsStream` +resolving an absolute name that did not match where the resources are +packaged), so both containers silently ran on stock server defaults. They +therefore measure **the tmpfs data dir alone**, not the server flags. + +Re-measured after that bug was fixed, with the flags verified as applied in the +running container (`innodb_buffer_pool_size=2147483648`, +`innodb_flush_log_at_trx_commit=0`, `innodb_doublewrite=OFF`, `log_bin=OFF`): + +| Task | tmpfs only | tmpfs + flags | Flags' marginal gain | +| ------------------------- | ---------- | ------------- | -------------------- | +| `core:coreMySQLTest` | 7m05s | 6m40s | ~6% | +| `core:corePostgreSQLTest` | 2m11s | 2m10s | ~0% (within noise) | + +So the headline 3-4x speedup comes almost entirely from moving the data dir to +`tmpfs`; the durability flags are a modest additional gain on top. Keep that in +mind before trading test fidelity for another flag -- the remaining headroom is +small. It is also why `--performance-schema=OFF` is not worth having: it would +buy a little overhead back while silently disabling the lock-contention +assertions described above. + +### Caveat: those numbers came from bigger hardware than CI + +The table above was measured on an **8 vCPU / 31 GB** development box. This +repo's actual lane -- the backend integration-test job in +`.github/workflows/backend-integration-test.yml` -- is `runs-on: ubuntu-22.04`, +a GitHub-hosted runner with **4 vCPU / 16 GiB RAM**. Do not expect to reproduce +3.73x there. + +Expect a real but smaller speedup. The win is **architectural**, not tied to a +specific machine: relaxed fsync/checkpoint semantics and a `tmpfs` data dir +remove I/O work that no amount of CPU would have made free. A smaller box has +less parallelism to exploit and less page cache, so the multiplier shrinks -- +the direction does not. + +## Headroom math: sizing the tmpfs caps + +Both `tmpfs` mounts are explicitly capped. This is required, not tidiness: an +uncapped `--tmpfs` defaults to **50% of host RAM per mount**, so two uncapped +mounts can claim 100% of RAM and get the Gradle daemon or a test fork +OOM-killed instead of failing cleanly. + +Use this formula to re-derive the caps for a different runner class: + +``` +safe tmpfs ceiling = total RAM + - build-process reserve (CoreDatabaseConcurrency + DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES) + - forks x (max heap + native overhead) + (TEST_WORKER_MAX_HEAP_MIB + + DEFAULT_TEST_WORKER_OVERHEAD_BYTES) + - database process RSS (innodb_buffer_pool_size / + shared_buffers, plus overhead) + - OS and Docker daemon +``` + +Worked example for the current `ubuntu-22.04` lane at `-PcoreDatabaseForks=2`: + +| Term | Size | +| ----------------------------------------------- | ------- | +| Total RAM | 16 GiB | +| Build-process reserve | -4 GiB | +| 2 forks x (4 GiB heap + 2 GiB native overhead) | -12 GiB | + +The JVM side alone nominally commits the entire box, so the mounts must be a +bounded worst case rather than an open-ended 8 GiB each. Real fork heap usage +sits well below the 4 GiB maximum, which is where the actual headroom comes +from -- but the *cap* has to assume it might not. + +Current caps: + +- **MySQL, `size=2g`.** The core test schema is small, and `--skip-log-bin` + plus the default (~100 MiB) redo capacity mean little besides table data + lands on the mount. The 2G buffer pool is separately resident as container + RSS, not on the tmpfs. +- **PostgreSQL, `size=5g`.** Deliberately larger, because `pg_wal` lives + *inside* the data dir and `postgresql.args` sets `max_wal_size=4GB` with + `checkpoint_timeout=1h`. Up to ~4 GiB of WAL can legitimately accumulate + before a checkpoint recycles it, so a 2g cap would turn a merely-late + checkpoint into an `ENOSPC` and a PostgreSQL `PANIC` mid-run. 5g = ~4 GiB WAL + headroom + ~1 GiB schema and data. + +Worst case both mounts total **7 GiB**, leaving ~9 GiB for the OS, both +database processes' RSS, and the fork JVMs' real heap usage. + +If you raise `max_wal_size` or `checkpoint_timeout`, raise the PostgreSQL cap +to match. If you move to a smaller runner, re-run the formula rather than +assuming these two numbers still fit. diff --git a/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/mysql.args b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/mysql.args new file mode 100644 index 00000000000..cf9d1460218 --- /dev/null +++ b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/mysql.args @@ -0,0 +1,52 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# Extra mysqld command-line flags for the shared MySQL test container +# (SharedDbContainerService). One token per line; blank lines and lines +# starting with # are ignored. +# +# Why these are safe HERE (and would NOT be safe in production): this +# container is disposable and is torn down at the end of every test run, +# so there is no need for crash durability, replication, or monitoring. +# +# Measured effect (apache/gravitino#13553, an 8 vCPU / 31 GB Linux machine, +# -PcoreDatabaseForks=2), core:coreMySQLTest wall clock: +# 26m25s per-fork containers (old design) +# 27m53s shared container, untuned +# 7m05s shared container + tmpfs data dir (flags NOT yet loading) +# 6m40s shared container + tmpfs data dir + the flags below, verified +# as actually applied inside the running container +# The staged numbers above predate the fix for a classpath bug that stopped +# this file from ever being read, so the large win is attributable to the tmpfs +# data dir; these flags add roughly a further 6%. See README.md next to this +# file for the full attribution table. +# All runs: 520 tests, 0 failures, 0 deadlocks / lock-wait timeouts. +# core:test (H2) is unchanged across all runs (1m27s -> 1m21s), the control +# confirming the speedup is database I/O and not the host. +# +# NOTE: --performance-schema is deliberately NOT disabled here, even though +# turning it off would shave a little more overhead. TestRoleMembershipWrites +# and TestOwnerAssignmentWrites assert on real row-lock contention by polling +# performance_schema.data_lock_waits; with the performance schema off that +# table is silently always empty, so those assertions would stop verifying +# anything without ever failing. +--innodb-flush-log-at-trx-commit=0 +--sync-binlog=0 +--innodb-buffer-pool-size=2G +--innodb-log-buffer-size=64M +--skip-log-bin +--innodb-doublewrite=0 diff --git a/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/postgresql.args b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/postgresql.args new file mode 100644 index 00000000000..a7a0548e262 --- /dev/null +++ b/buildSrc/src/main/resources/org/apache/gravitino/testing/shared-db-tuning/postgresql.args @@ -0,0 +1,44 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# Extra postgres server args for the shared PostgreSQL test container +# (SharedDbContainerService). One token per line; each pair of +# '-c' / 'key=value' lines becomes one '-c key=value' server arg. Blank +# lines and lines starting with # are ignored. +# +# Same rationale as mysql.args: this container is disposable and torn down +# at the end of every test run, so relaxing crash-durability guarantees and +# skipping long-term maintenance work (autovacuum) is safe here and would +# NOT be safe in production. +-c +fsync=off +-c +synchronous_commit=off +-c +full_page_writes=off +-c +shared_buffers=1GB +-c +max_wal_size=4GB +-c +checkpoint_timeout=1h +-c +autovacuum=off +-c +wal_level=minimal +-c +max_wal_senders=0 diff --git a/buildSrc/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseConcurrency.java b/buildSrc/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseConcurrency.java new file mode 100644 index 00000000000..627391d0b25 --- /dev/null +++ b/buildSrc/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseConcurrency.java @@ -0,0 +1,300 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +import static org.apache.gravitino.testing.CoreDatabaseConcurrency.DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES; +import static org.apache.gravitino.testing.CoreDatabaseConcurrency.DEFAULT_TEST_WORKER_OVERHEAD_BYTES; +import static org.apache.gravitino.testing.CoreDatabaseConcurrency.TEST_WORKER_MAX_HEAP_BYTES; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.gravitino.testing.CoreDatabaseConcurrency.Budget; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.ConfigurationSource; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.DetectedCapacity; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.LimitingFactor; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.MemoryConfiguration; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.Resolution; +import org.apache.gravitino.testing.CoreDatabaseConcurrency.Source; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +class TestCoreDatabaseConcurrency { + + private static final long MIB = 1024L * 1024L; + private static final long GIB = 1024L * MIB; + + @Test + void usesDefaultMemoryConfiguration() { + MemoryConfiguration configuration = + CoreDatabaseConcurrency.resolveMemoryConfiguration(null, null, null, null); + + assertEquals(DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES, configuration.buildReserveBytes()); + assertEquals(ConfigurationSource.DEFAULT, configuration.buildReserveSource()); + assertEquals(DEFAULT_TEST_WORKER_OVERHEAD_BYTES, configuration.forkOverheadBytes()); + assertEquals(ConfigurationSource.DEFAULT, configuration.forkOverheadSource()); + } + + @Test + void memoryPropertiesTakePrecedenceOverEnvironment() { + MemoryConfiguration configuration = + CoreDatabaseConcurrency.resolveMemoryConfiguration("5120", "6144", "1024", "2048"); + + assertEquals(5120L * MIB, configuration.buildReserveBytes()); + assertEquals(ConfigurationSource.PROPERTY, configuration.buildReserveSource()); + assertEquals(1024L * MIB, configuration.forkOverheadBytes()); + assertEquals(ConfigurationSource.PROPERTY, configuration.forkOverheadSource()); + } + + @Test + void memoryEnvironmentIsUsedWhenPropertiesAreAbsent() { + MemoryConfiguration configuration = + CoreDatabaseConcurrency.resolveMemoryConfiguration(null, "5120", null, "1024"); + + assertEquals(5120L * MIB, configuration.buildReserveBytes()); + assertEquals(ConfigurationSource.ENVIRONMENT, configuration.buildReserveSource()); + assertEquals(1024L * MIB, configuration.forkOverheadBytes()); + assertEquals(ConfigurationSource.ENVIRONMENT, configuration.forkOverheadSource()); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "zero", "0", "-1", "1.5", "9223372036854775807"}) + void rejectsInvalidConfiguredMemory(String value) { + assertThrows( + IllegalArgumentException.class, + () -> CoreDatabaseConcurrency.resolveMemoryConfiguration(value, null, null, null)); + assertThrows( + IllegalArgumentException.class, + () -> CoreDatabaseConcurrency.resolveMemoryConfiguration(null, null, value, null)); + } + + @Test + void rolloutBudgetIncludesHeapAndNativeAllowance() { + MemoryConfiguration configuration = + CoreDatabaseConcurrency.resolveMemoryConfiguration(null, null, null, null); + Budget budget = CoreDatabaseConcurrency.rolloutBudget(configuration); + + assertEquals( + TEST_WORKER_MAX_HEAP_BYTES + DEFAULT_TEST_WORKER_OVERHEAD_BYTES, + budget.memoryBytesPerFork()); + assertEquals(DEFAULT_BUILD_PROCESS_MEMORY_RESERVE_BYTES, budget.buildReserveBytes()); + assertEquals(2, budget.rolloutCap()); + } + + @Test + void rejectsOverflowingRolloutBudget() { + MemoryConfiguration configuration = + new MemoryConfiguration( + GIB, ConfigurationSource.DEFAULT, Long.MAX_VALUE, ConfigurationSource.PROPERTY); + + assertThrows( + IllegalArgumentException.class, () -> CoreDatabaseConcurrency.rolloutBudget(configuration)); + } + + @Test + void calculatesAvailableMemoryWithoutUnderflow() { + assertEquals( + 12L * GIB, CoreDatabaseConcurrency.availableMemoryForTestWorkers(16L * GIB, 4L * GIB)); + assertEquals(0, CoreDatabaseConcurrency.availableMemoryForTestWorkers(2L * GIB, 4L * GIB)); + assertEquals(0, CoreDatabaseConcurrency.availableMemoryForTestWorkers(0, 4L * GIB)); + } + + @Test + void fullyDetectedCapacityEnablesTwoForkRollout() { + Resolution resolution = resolve(null, null, new DetectedCapacity(8, 64L * GIB, 8, 1), 0); + + assertEquals(2, resolution.forks()); + assertEquals(Source.DETECTED, resolution.source()); + assertEquals(LimitingFactor.ROLLOUT, resolution.limitingFactor()); + assertEquals(8, resolution.safeMaximum()); + assertEquals(2, resolution.allowedMaximum()); + } + + @Test + void cpuCanLimitForks() { + Resolution resolution = resolve(null, null, new DetectedCapacity(1, 64L * GIB, 8, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(LimitingFactor.CPU, resolution.limitingFactor()); + } + + @Test + void memoryCanLimitForks() { + Resolution resolution = resolve(null, null, new DetectedCapacity(8, 10L * GIB, 8, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(LimitingFactor.MEMORY, resolution.limitingFactor()); + } + + @Test + void gradleWorkersCanLimitForks() { + Resolution resolution = resolve(null, null, new DetectedCapacity(8, 64L * GIB, 1, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(LimitingFactor.GRADLE, resolution.limitingFactor()); + } + + @Test + void selectedLaneCountDividesDetectedCapacity() { + Resolution resolution = resolve(null, null, new DetectedCapacity(2, 16L * GIB, 2, 2), 0); + + assertEquals(1, resolution.forks()); + assertEquals(1, resolution.safeMaximum()); + assertEquals(LimitingFactor.CPU, resolution.limitingFactor()); + } + + @Test + void incompleteSignalsFallBackToOneFork() { + Resolution resolution = resolve(null, null, new DetectedCapacity(8, 0, 8, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(Source.DEFAULT, resolution.source()); + assertEquals(LimitingFactor.DEFAULT, resolution.limitingFactor()); + assertEquals(8, resolution.safeMaximum()); + } + + @Test + void absentSignalsFallBackToOneFork() { + Resolution resolution = resolve(null, null, new DetectedCapacity(0, 0, 0, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(Source.DEFAULT, resolution.source()); + assertEquals(0, resolution.safeMaximum()); + assertEquals(2, resolution.allowedMaximum()); + } + + @Test + void propertyOverrideTakesPrecedence() { + Resolution resolution = resolve("1", "2", new DetectedCapacity(8, 64L * GIB, 8, 1), 0); + + assertEquals(1, resolution.forks()); + assertEquals(Source.PROPERTY, resolution.source()); + } + + @Test + void environmentOverrideIsUsedWhenPropertyIsAbsent() { + Resolution resolution = resolve(null, "2", new DetectedCapacity(8, 64L * GIB, 8, 1), 0); + + assertEquals(2, resolution.forks()); + assertEquals(Source.ENVIRONMENT, resolution.source()); + } + + @Test + void exactOverrideCanUseUnknownCapacityWithinKnownLimits() { + Resolution resolution = resolve("2", null, new DetectedCapacity(0, 0, 0, 1), 0); + + assertEquals(2, resolution.forks()); + assertEquals(Source.PROPERTY, resolution.source()); + assertEquals(2, resolution.allowedMaximum()); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "zero", "0", "-1", "1.5", "2147483648"}) + void rejectsMalformedExactOverrides(String value) { + assertThrows( + IllegalArgumentException.class, + () -> resolve(value, null, new DetectedCapacity(8, 64L * GIB, 8, 1), 0)); + } + + @Test + void rejectsOverrideAboveRolloutCap() { + IllegalArgumentException failure = + assertThrows( + IllegalArgumentException.class, + () -> resolve("3", null, new DetectedCapacity(8, 64L * GIB, 8, 1), 0)); + + assertTrue(failure.getMessage().contains("allowed maximum is 2")); + } + + @Test + void rejectsOverrideAboveKnownPartialCapacity() { + assertThrows( + IllegalArgumentException.class, + () -> resolve("2", null, new DetectedCapacity(1, 0, 0, 1), 0)); + } + + @Test + void rejectsOverrideAbovePerLaneCapacity() { + assertThrows( + IllegalArgumentException.class, + () -> resolve("2", null, new DetectedCapacity(2, 16L * GIB, 2, 2), 0)); + } + + @Test + void fixedNetworkDockerLaneForcesOneFork() { + int laneMaximum = CoreDatabaseConcurrency.laneMaximum(true, true); + Resolution resolution = + resolve(null, null, new DetectedCapacity(8, 64L * GIB, 8, 1), laneMaximum); + + assertEquals(1, resolution.forks()); + assertEquals(LimitingFactor.LANE, resolution.limitingFactor()); + assertThrows( + IllegalArgumentException.class, + () -> resolve("2", null, new DetectedCapacity(8, 64L * GIB, 8, 1), laneMaximum)); + } + + @Test + void h2AndNonFixedNetworkDockerLanesHaveNoSpecialMaximum() { + assertEquals(0, CoreDatabaseConcurrency.laneMaximum(false, true)); + assertEquals(0, CoreDatabaseConcurrency.laneMaximum(true, false)); + } + + @Test + void rejectsInvalidCapacityAndBudgetInputs() { + Budget budget = defaultBudget(); + assertThrows( + IllegalArgumentException.class, + () -> + CoreDatabaseConcurrency.resolve( + null, null, new DetectedCapacity(-1, 1, 1, 1), budget, 0)); + assertThrows( + IllegalArgumentException.class, + () -> + CoreDatabaseConcurrency.resolve( + null, null, new DetectedCapacity(1, -1, 1, 1), budget, 0)); + assertThrows( + IllegalArgumentException.class, + () -> + CoreDatabaseConcurrency.resolve( + null, null, new DetectedCapacity(1, 1, -1, 1), budget, 0)); + assertThrows( + IllegalArgumentException.class, + () -> + CoreDatabaseConcurrency.resolve( + null, null, new DetectedCapacity(1, 1, 1, 0), budget, 0)); + assertThrows( + IllegalArgumentException.class, + () -> + CoreDatabaseConcurrency.resolve( + null, null, new DetectedCapacity(1, 1, 1, 1), budget, -1)); + } + + private static Resolution resolve( + String propertyValue, String environmentValue, DetectedCapacity capacity, int laneMaximum) { + return CoreDatabaseConcurrency.resolve( + propertyValue, environmentValue, capacity, defaultBudget(), laneMaximum); + } + + private static Budget defaultBudget() { + return CoreDatabaseConcurrency.rolloutBudget( + CoreDatabaseConcurrency.resolveMemoryConfiguration(null, null, null, null)); + } +} diff --git a/buildSrc/src/test/java/org/apache/gravitino/testing/TestSharedDbContainerService.java b/buildSrc/src/test/java/org/apache/gravitino/testing/TestSharedDbContainerService.java new file mode 100644 index 00000000000..9fa7ca5627c --- /dev/null +++ b/buildSrc/src/test/java/org/apache/gravitino/testing/TestSharedDbContainerService.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.testing; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link SharedDbContainerService}'s pure parsing/liveness logic. Container + * lifecycle itself (starting a container, readiness polling, stale-container removal) needs a + * real Docker daemon and is exercised by the Core database Gradle lanes instead. + */ +public class TestSharedDbContainerService { + + @Test + public void testParseHostPortSingleLine() { + assertEquals(49153, SharedDbContainerService.parseHostPort("0.0.0.0:49153", "abc123")); + } + + @Test + public void testParseHostPortUsesFirstLineWhenMultipleAddressFamiliesPublished() { + // `docker port` prints one line per address family when the daemon also publishes IPv6. + assertEquals( + 49153, + SharedDbContainerService.parseHostPort("0.0.0.0:49153\n[::]:49153", "abc123")); + } + + @Test + public void testParseHostPortTrimsWhitespace() { + assertEquals(49153, SharedDbContainerService.parseHostPort(" 0.0.0.0:49153 \n", "abc123")); + } + + @Test + public void testParseHostPortThrowsOnEmptyOutput() { + IllegalStateException e = + assertThrows( + IllegalStateException.class, + () -> SharedDbContainerService.parseHostPort("", "abc123")); + assertTrue(e.getMessage().contains("abc123")); + } + + @Test + public void testParseHostPortThrowsOnNonNumericPort() { + assertThrows( + NumberFormatException.class, + () -> SharedDbContainerService.parseHostPort("0.0.0.0:notaport", "abc123")); + } + + @Test + public void testSplitNonBlankLinesFiltersBlanksAndTrims() { + List lines = SharedDbContainerService.splitNonBlankLines(" a \n\n b\n \nc"); + assertEquals(List.of("a", "b", "c"), lines); + } + + @Test + public void testSplitNonBlankLinesEmptyInputReturnsEmptyList() { + assertTrue(SharedDbContainerService.splitNonBlankLines("").isEmpty()); + assertTrue(SharedDbContainerService.splitNonBlankLines(" \n \n").isEmpty()); + } + + @Test + public void testIsOwningProcessAliveTrueForCurrentProcess() { + assertTrue( + SharedDbContainerService.isOwningProcessAlive( + String.valueOf(ProcessHandle.current().pid()))); + } + + @Test + public void testIsOwningProcessAliveFalseForUnusedPid() { + // PID 0 is reserved/invalid on essentially every OS Gradle runs on, and is never a real, + // live user process - a safe stand-in for "definitely not alive" without depending on any + // specific dead PID happening to be free at test time. + assertFalse(SharedDbContainerService.isOwningProcessAlive("0")); + } + + @Test + public void testIsOwningProcessAliveFalseForUnparsableLabel() { + // A missing/malformed PID label (e.g. a container from before this label existed) must bias + // toward "not alive" so removeStaleContainers() reaps it rather than leaking it forever. + assertFalse(SharedDbContainerService.isOwningProcessAlive("")); + assertFalse(SharedDbContainerService.isOwningProcessAlive("not-a-pid")); + } +} diff --git a/core/build.gradle.kts b/core/build.gradle.kts index bdd8cbbfc1c..dc760648802 100644 --- a/core/build.gradle.kts +++ b/core/build.gradle.kts @@ -1,4 +1,11 @@ +import com.sun.management.OperatingSystemMXBean import net.ltgt.gradle.errorprone.errorprone +import org.apache.gravitino.testing.CoreDatabaseConcurrency +import org.apache.gravitino.testing.CoreDatabaseConcurrency.DetectedCapacity +import org.apache.gravitino.testing.DbConnectionInfo +import org.apache.gravitino.testing.SharedDbContainerService +import org.gradle.api.provider.Provider +import java.lang.management.ManagementFactory /* * Licensed to the Apache Software Foundation (ASF) under one @@ -105,17 +112,293 @@ artifacts { add("testArtifacts", testJar) } +// Core's tests run in three tasks: `test` (unit tests and H2, no Docker), coreMySQLTest and +// corePostgreSQLTest. A class joins a backend through its @CoreBackend.* annotation; see +// CoreBackend.java. +// Backend name -> the JUnit tag that admits a class to that lane. Keep in sync with CoreBackend. +val coreBackendTestTags = + linkedMapOf( + "h2" to "gravitino-core-h2-test", + "mysql" to "gravitino-core-mysql-test", + "postgresql" to "gravitino-core-postgresql-test" + ) +val coreTestBackendProperty = "gravitino.core.test.backend" +val coreDatabaseTaskNames = + setOf("coreMySQLTest", "corePostgreSQLTest") +// How many database tasks this build may run at once: those named explicitly, or both when +// `check`/`build` pulls them in with Docker tests on. +val coreDatabaseRequestedTasks = gradle.startParameter.taskNames.map { it.substringAfterLast(':') } +val coreDatabaseActiveLaneCount = + if ( + rootProject.extra["dockerTest"] == true && + coreDatabaseRequestedTasks.any { it == "check" || it == "build" } + ) { + coreDatabaseTaskNames.size + } else { + coreDatabaseRequestedTasks.distinct().count { it in coreDatabaseTaskNames }.coerceAtLeast(1) + } +val coreDatabaseTotalMemoryBytes = + runCatching { + (ManagementFactory.getOperatingSystemMXBean() as? OperatingSystemMXBean)?.totalMemorySize ?: 0L + }.getOrDefault(0L) +val coreDatabaseMemoryBasis = + if (coreDatabaseTotalMemoryBytes > 0) { + "jdk-total-memory-minus-configured-build-reserve" + } else { + "unavailable" + } +val coreDatabaseMemoryConfiguration = + CoreDatabaseConcurrency.resolveMemoryConfiguration( + providers.gradleProperty(CoreDatabaseConcurrency.BUILD_MEMORY_RESERVE_PROPERTY).orNull, + providers + .environmentVariable(CoreDatabaseConcurrency.BUILD_MEMORY_RESERVE_ENVIRONMENT_VARIABLE) + .orNull, + providers.gradleProperty(CoreDatabaseConcurrency.FORK_MEMORY_OVERHEAD_PROPERTY).orNull, + providers + .environmentVariable(CoreDatabaseConcurrency.FORK_MEMORY_OVERHEAD_ENVIRONMENT_VARIABLE) + .orNull + ) +val coreDatabaseAvailableMemoryBytes = + CoreDatabaseConcurrency.availableMemoryForTestWorkers( + coreDatabaseTotalMemoryBytes, + coreDatabaseMemoryConfiguration.buildReserveBytes() + ) +val coreDatabaseDetectedCapacity = + DetectedCapacity( + Runtime.getRuntime().availableProcessors(), + coreDatabaseTotalMemoryBytes, + gradle.startParameter.maxWorkerCount, + coreDatabaseActiveLaneCount + ) +val coreDatabaseConcurrencyBudget = + CoreDatabaseConcurrency.rolloutBudget(coreDatabaseMemoryConfiguration) +val macDockerConnectorFixedNetwork = + rootProject.extra[ + CoreDatabaseConcurrency.MAC_DOCKER_CONNECTOR_FIXED_NETWORK_EXTRA + ] as? Boolean ?: false + +// Connection pool budget for the shared-container path: several test-JVM forks may briefly hold +// connections against the same physical server at once, so this stays well under MySQL's default +// max_connections=151 / PostgreSQL's default max_connections=100 even with a handful of forks and +// classes active concurrently, while remaining far larger than any single test actually needs. +val sharedDbTestConnectionPoolSize = 20 + +// A database the environment injects for a backend, e.g. a service container on a Kubernetes +// runner: GRAVITINO_TEST_MYSQL_URL / _USER / _PASSWORD (or POSTGRESQL). The URL is an admin JDBC +// URL; each test fixture gets its own new database on that server. When set, the task uses it +// and starts no container; otherwise it falls back to Docker. +data class InjectedDatabase(val url: String, val user: String, val password: String) + +fun injectedDatabase(backend: String): InjectedDatabase? { + val prefix = "GRAVITINO_TEST_${backend.uppercase()}_" + val url = providers.environmentVariable(prefix + "URL").orNull?.takeIf { it.isNotBlank() } ?: return null + return InjectedDatabase( + url, + providers.environmentVariable(prefix + "USER").orNull ?: "root", + providers.environmentVariable(prefix + "PASSWORD").orNull ?: "" + ) +} + +fun registerCoreTestTask( + taskName: String, + backend: String +) = tasks.register(taskName) { + group = "verification" + description = "Runs core database tests against $backend." + + testClassesDirs = sourceSets["test"].output.classesDirs + classpath = sourceSets["test"].runtimeClasspath + + reports.html.outputLocation.set( + rootProject.layout.buildDirectory.dir("reports/tests/core/$taskName") + ) + + useJUnitPlatform { + // Plain tag include, applied by JUnit at discovery time, so classes not tagged for this + // backend never show up in this task's JUnit XML. A class tagged for several backends runs + // under each of them. + includeTags( + coreBackendTestTags[backend] ?: throw GradleException("Unsupported core test backend: $backend") + ) + } + + val shardTelemetryDirectory = + layout.buildDirectory.dir("test-results/$taskName/shard-telemetry") + val laneMaximum = CoreDatabaseConcurrency.laneMaximum(true, macDockerConnectorFixedNetwork) + val concurrencyResolution = + CoreDatabaseConcurrency.resolve( + providers.gradleProperty(CoreDatabaseConcurrency.FORKS_PROPERTY).orNull, + providers.environmentVariable(CoreDatabaseConcurrency.FORKS_ENVIRONMENT_VARIABLE).orNull, + coreDatabaseDetectedCapacity, + coreDatabaseConcurrencyBudget, + laneMaximum + ) + + systemProperty(coreTestBackendProperty, backend) + // Read by the root setTestEnvironment inside doFirst, i.e. at execution time, after every + // configuration action has run - so it doesn't depend on register/configureEach ordering. + extensions.extraProperties["includeDockerTaggedTests"] = true + + // Each Gradle worker owns its database server. JUnit execution remains serial inside it. + maxParallelForks = concurrencyResolution.forks() + systemProperty("junit.jupiter.extensions.autodetection.enabled", "true") + systemProperty("junit.jupiter.execution.parallel.enabled", "false") + systemProperty( + "gravitino.core.database.shard.telemetry.directory", + shardTelemetryDirectory.get().asFile.absolutePath + ) + outputs.dir(shardTelemetryDirectory) + + doFirst { + project.delete(shardTelemetryDirectory) + logger.lifecycle( + "[CORE-DB-CONCURRENCY] task={} forks={} source={} limitingFactor={} " + + "safeMaximum={} allowedMaximum={} processors={} totalMemoryBytes={} " + + "availableTestMemoryBytes={} memoryBasis={} buildReserveBytes={} " + + "buildReserveSource={} memoryBytesPerFork={} testWorkerMaxHeapBytes={} " + + "forkOverheadBytes={} forkOverheadSource={} gradleMaxWorkers={} " + + "activeDatabaseLanes={} rolloutCap={} laneMaximum={} " + + "macDockerConnectorFixedNetwork={} junitParallel=false", + path, + concurrencyResolution.forks(), + concurrencyResolution.source(), + concurrencyResolution.limitingFactor(), + concurrencyResolution.safeMaximum(), + concurrencyResolution.allowedMaximum(), + coreDatabaseDetectedCapacity.processors(), + coreDatabaseTotalMemoryBytes, + coreDatabaseAvailableMemoryBytes, + coreDatabaseMemoryBasis, + coreDatabaseMemoryConfiguration.buildReserveBytes(), + coreDatabaseMemoryConfiguration.buildReserveSource(), + coreDatabaseConcurrencyBudget.memoryBytesPerFork(), + CoreDatabaseConcurrency.TEST_WORKER_MAX_HEAP_BYTES, + coreDatabaseMemoryConfiguration.forkOverheadBytes(), + coreDatabaseMemoryConfiguration.forkOverheadSource(), + coreDatabaseDetectedCapacity.gradleMaxWorkers(), + coreDatabaseDetectedCapacity.activeDatabaseLanes(), + coreDatabaseConcurrencyBudget.rolloutCap(), + laneMaximum, + macDockerConnectorFixedNetwork + ) + } + + doLast { + val shardRecords = + shardTelemetryDirectory + .get() + .asFile + .walkTopDown() + .filter { it.isFile && it.extension == "log" } + .flatMap { it.readLines().asSequence() } + .filter { it.startsWith("[CORE-DB-SHARD]") } + .sorted() + .toList() + if (shardRecords.isEmpty()) { + throw GradleException("The $backend core database lane emitted no shard telemetry.") + } + shardRecords.forEach { logger.lifecycle(it) } + } + + val injected = injectedDatabase(backend) + doFirst { + if (injected == null && rootProject.extra["dockerTest"] != true) { + throw GradleException( + "$path requires Docker or an injected database; use -PskipDockerTests=false with " + + "Docker running, or set GRAVITINO_TEST_${backend.uppercase()}_URL." + ) + } + // Which server this task uses; CI's injected-database job asserts it. + val source = + if (injected != null) "injected" else if (rootProject.extra["dockerTest"] == true) "docker" else "none" + logger.lifecycle("[CORE-DB-SOURCE] task={} backend={} source={}", path, backend, source) + } + + // Contract with SharedCoreDatabaseProvisioner (core/src/test/java/.../storage/relational) + // and its ADMIN_PROPERTY_PREFIX/POOL_MAX_CONNECTIONS_PROPERTY constants - kept as literal + // strings on both sides rather than importing that class here, since a main build script + // cannot see a project's own test-source classes at configuration time (they are compiled + // by a task, not on the script's own classpath). + val sharedDbAdminPropertyPrefix = "gravitino.test.db." + systemProperty(sharedDbAdminPropertyPrefix + "pool.maxConnections", sharedDbTestConnectionPoolSize) + + // Must match root build.gradle.kts's own dockerTest wiring exactly (rootProject.extra + // only, not e.g. System.getenv("dockerTest")): it unconditionally sets this task's + // dockerTest environment variable from rootProject.extra["dockerTest"], overriding + // anything a developer exported in their shell, which is what BackendTestExtension + // actually reads at test-JVM runtime. Gating shared-container startup on anything looser + // here could start it even when the forked test JVM will never see dockerTest=true and so + // never use it. + if (injected != null) { + // Set at execution time so credentials stay out of the task's cache key. + doFirst { + systemProperty(sharedDbAdminPropertyPrefix + backend + ".adminUrl", injected.url) + systemProperty(sharedDbAdminPropertyPrefix + backend + ".user", injected.user) + systemProperty(sharedDbAdminPropertyPrefix + backend + ".password", injected.password) + } + } else if (rootProject.extra["dockerTest"] == true) { + @Suppress("UNCHECKED_CAST") + val sharedDbContainerService = + rootProject.extra["sharedDbContainerService"] as Provider + usesService(sharedDbContainerService) + + doFirst { + val connectionInfo: DbConnectionInfo = sharedDbContainerService.get().connectionInfo(backend) + systemProperty(sharedDbAdminPropertyPrefix + backend + ".adminUrl", connectionInfo.adminJdbcUrl) + systemProperty(sharedDbAdminPropertyPrefix + backend + ".user", connectionInfo.user) + systemProperty(sharedDbAdminPropertyPrefix + backend + ".password", connectionInfo.password) + } + } +} + +registerCoreTestTask("coreMySQLTest", "mysql") +registerCoreTestTask("corePostgreSQLTest", "postgresql") + +// `test` runs everything core can run without Docker: unit tests (no backend tag) and classes +// tagged for H2, against H2. Classes tagged only for MySQL or PostgreSQL, and Docker-tagged +// classes with no backend tag (see TestCoreLaneMembership), stay out. One JVM, as before the split. tasks.test { - val policiesDocument = rootProject.file("docs/policies.md") - inputs.file(policiesDocument) + // TestRestrictedRegoGoldenFile reads docs/policies.md. + inputs.file(rootProject.file("docs/policies.md")) systemProperty("gravitino.repositoryRoot", rootProject.projectDir.path) - - val testMode = project.properties["testMode"] as? String ?: "embedded" - if (testMode == "embedded") { - environment("GRAVITINO_HOME", project.rootDir.path) - } else { - environment("GRAVITINO_HOME", project.rootDir.path + "/distribution/package") + systemProperty(coreTestBackendProperty, "h2") + // Read by the root setTestEnvironment inside doFirst; lets H2 run the Docker-tagged backend + // classes, which need no container on H2. + extensions.extraProperties["includeDockerTaggedTests"] = true + useJUnitPlatform { + includeTags( + "${coreBackendTestTags.getValue("h2")} | " + + "!(${coreBackendTestTags.getValue("mysql")} | " + + "${coreBackendTestTags.getValue("postgresql")} | gravitino-docker-test)" + ) } + maxParallelForks = 1 + systemProperty("junit.jupiter.execution.parallel.enabled", "false") +} + +val coreDatabaseTestTasks = listOf("coreMySQLTest", "corePostgreSQLTest") +// `check` (and so `build`) also runs a database task when Docker tests are on or its database is +// injected. They run no +// tests under -PskipTests, which selects integration tests only and core has none. +tasks.check { + dependsOn( + coreDatabaseTestTasks.filter { + rootProject.extra["dockerTest"] == true || + injectedDatabase(it.removePrefix("core").removeSuffix("Test").lowercase()) != null + } + ) +} +coreDatabaseTestTasks.forEach { + tasks.named(it) { onlyIf("-PskipTests selects integration tests only") { !rootProject.hasProperty("skipTests") } } +} + +// One core report from the three tasks' exec files; missing ones (e.g. no Docker run) are skipped. +tasks.named("jacocoTestReport") { + // `build` with Docker runs the database tasks too; report only after they finish. + mustRunAfter(coreDatabaseTestTasks) + executionData.setFrom( + (listOf("test") + coreDatabaseTestTasks).map { layout.buildDirectory.file("jacoco/$it.exec") } + ) } tasks.withType().configureEach { diff --git a/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java b/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java index 487645a10d9..8336560691f 100644 --- a/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java +++ b/core/src/test/java/org/apache/gravitino/job/TestJobManagerMultiNode.java @@ -51,6 +51,8 @@ import org.apache.gravitino.meta.JobEntity; import org.apache.gravitino.meta.JobTemplateEntity; import org.apache.gravitino.storage.RandomIdGenerator; +import org.apache.gravitino.storage.relational.DatabaseFixture; +import org.apache.gravitino.storage.relational.DatabaseIsolation; import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.utils.NameIdentifierUtil; @@ -66,6 +68,7 @@ * JobManager} and {@link LocalJobExecutor}, share the same relational metadata store. The status * pull and the cleanup are triggered manually, so that the test controls which node runs them when. */ +@DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) public class TestJobManagerMultiNode extends TestJDBCBackend { private static final String METALAKE = "metalake_job_multi_node"; diff --git a/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java b/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java index b1cac18a986..a760ad9ec3f 100644 --- a/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java +++ b/core/src/test/java/org/apache/gravitino/stats/storage/TestJdbcPartitionStatisticStorageIT.java @@ -50,6 +50,7 @@ import org.apache.gravitino.integration.test.container.MySQLContainer; import org.apache.gravitino.integration.test.container.PGImageName; import org.apache.gravitino.integration.test.container.PostgreSQLContainer; +import org.apache.gravitino.integration.test.util.CloseContainerExtension; import org.apache.gravitino.integration.test.util.TestDatabaseName; import org.apache.gravitino.meta.TableEntity; import org.apache.gravitino.stats.PartitionRange; @@ -58,12 +59,15 @@ import org.apache.gravitino.stats.PartitionStatisticsUpdate; import org.apache.gravitino.stats.StatisticValue; import org.apache.gravitino.stats.StatisticValues; +import org.apache.gravitino.storage.relational.CoreBackend; +import org.apache.gravitino.storage.relational.SharedCoreDatabaseProvisioner; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,9 +97,11 @@ public class TestJdbcPartitionStatisticStorageIT { /** * Abstract base class containing all test logic. Each database-specific test class extends this - * and implements the database setup. + * and implements the database setup. Each subclass carries the tag of the one backend lane it + * runs in. */ @TestInstance(TestInstance.Lifecycle.PER_CLASS) + @ExtendWith(CloseContainerExtension.class) abstract static class BaseJdbcPartitionStatisticStorageTest { protected JdbcPartitionStatisticStorage storage; @@ -583,32 +589,45 @@ protected void cleanupAllStatistics() throws IOException { } } - /** MySQL-specific tests using Docker container. */ + /** MySQL-specific tests, on the shared or injected server, or else a Docker container. */ @Nested + @CoreBackend.MySQL @Tag("gravitino-docker-test") static class MySQLTest extends BaseJdbcPartitionStatisticStorageTest { private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); private static final TestDatabaseName TEST_DB_NAME = TestDatabaseName.MYSQL_MYSQL_ABSTRACT_IT; - private MySQLContainer mySQLContainer; + private String jdbcUrl; + private String username; + private String password; + private String driver; @Override protected void setupStorage() throws Exception { - LOG.info("Starting MySQL container for partition statistics integration tests"); - - // Start MySQL container - containerSuite.startMySQLContainer(TEST_DB_NAME); - mySQLContainer = containerSuite.getMySQLContainer(); + if (SharedCoreDatabaseProvisioner.isEnabled("mysql")) { + LOG.info("Using the shared MySQL server for partition statistics integration tests"); + jdbcUrl = SharedCoreDatabaseProvisioner.createDatabase("mysql"); + username = SharedCoreDatabaseProvisioner.user("mysql"); + password = SharedCoreDatabaseProvisioner.password("mysql"); + driver = "com.mysql.cj.jdbc.Driver"; + } else { + LOG.info("Starting MySQL container for partition statistics integration tests"); + containerSuite.startMySQLContainer(TEST_DB_NAME); + MySQLContainer mySQLContainer = containerSuite.getMySQLContainer(); + jdbcUrl = mySQLContainer.getJdbcUrl(TEST_DB_NAME); + username = mySQLContainer.getUsername(); + password = mySQLContainer.getPassword(); + driver = mySQLContainer.getDriverClassName(TEST_DB_NAME); + } // Create database schema createMySQLSchema(); - // Create storage factory with MySQL container connection Map properties = Maps.newHashMap(); - properties.put("jdbcUrl", mySQLContainer.getJdbcUrl(TEST_DB_NAME)); - properties.put("jdbcUser", mySQLContainer.getUsername()); - properties.put("jdbcPassword", mySQLContainer.getPassword()); - properties.put("jdbcDriver", mySQLContainer.getDriverClassName(TEST_DB_NAME)); + properties.put("jdbcUrl", jdbcUrl); + properties.put("jdbcUser", username); + properties.put("jdbcPassword", password); + properties.put("jdbcDriver", driver); JdbcPartitionStatisticStorageFactory factory = new JdbcPartitionStatisticStorageFactory(); storage = (JdbcPartitionStatisticStorage) factory.create(properties); @@ -626,10 +645,6 @@ protected void teardownStorage() throws Exception { /** Creates the partition_statistic_meta table in the MySQL test database. */ private void createMySQLSchema() throws SQLException { - String jdbcUrl = mySQLContainer.getJdbcUrl(TEST_DB_NAME); - String username = mySQLContainer.getUsername(); - String password = mySQLContainer.getPassword(); - try (Connection conn = DriverManager.getConnection(jdbcUrl, username, password); Statement stmt = conn.createStatement()) { @@ -653,32 +668,46 @@ private void createMySQLSchema() throws SQLException { } } - /** PostgreSQL-specific tests using Docker container. */ + /** PostgreSQL-specific tests, on the shared or injected server, or else a Docker container. */ @Nested + @CoreBackend.PostgreSQL @Tag("gravitino-docker-test") static class PostgreSQLTest extends BaseJdbcPartitionStatisticStorageTest { private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); private static final TestDatabaseName TEST_DB_NAME = TestDatabaseName.PG_TEST_PARTITION_STATS; - private PostgreSQLContainer postgreSQLContainer; + private String jdbcUrl; + private String username; + private String password; + private String driver; @Override protected void setupStorage() throws Exception { - LOG.info("Starting PostgreSQL container for partition statistics integration tests"); - - // Start PostgreSQL container - containerSuite.startPostgreSQLContainer(TEST_DB_NAME, PGImageName.VERSION_13); - postgreSQLContainer = containerSuite.getPostgreSQLContainer(PGImageName.VERSION_13); + if (SharedCoreDatabaseProvisioner.isEnabled("postgresql")) { + LOG.info("Using the shared PostgreSQL server for partition statistics integration tests"); + jdbcUrl = SharedCoreDatabaseProvisioner.createDatabase("postgresql"); + username = SharedCoreDatabaseProvisioner.user("postgresql"); + password = SharedCoreDatabaseProvisioner.password("postgresql"); + driver = "org.postgresql.Driver"; + } else { + LOG.info("Starting PostgreSQL container for partition statistics integration tests"); + containerSuite.startPostgreSQLContainer(TEST_DB_NAME, PGImageName.VERSION_13); + PostgreSQLContainer postgreSQLContainer = + containerSuite.getPostgreSQLContainer(PGImageName.VERSION_13); + jdbcUrl = postgreSQLContainer.getJdbcUrl(TEST_DB_NAME); + username = postgreSQLContainer.getUsername(); + password = postgreSQLContainer.getPassword(); + driver = postgreSQLContainer.getDriverClassName(TEST_DB_NAME); + } // Create database schema createPostgreSQLSchema(); - // Create storage factory with PostgreSQL container connection Map properties = Maps.newHashMap(); - properties.put("jdbcUrl", postgreSQLContainer.getJdbcUrl(TEST_DB_NAME)); - properties.put("jdbcUser", postgreSQLContainer.getUsername()); - properties.put("jdbcPassword", postgreSQLContainer.getPassword()); - properties.put("jdbcDriver", postgreSQLContainer.getDriverClassName(TEST_DB_NAME)); + properties.put("jdbcUrl", jdbcUrl); + properties.put("jdbcUser", username); + properties.put("jdbcPassword", password); + properties.put("jdbcDriver", driver); JdbcPartitionStatisticStorageFactory factory = new JdbcPartitionStatisticStorageFactory(); storage = (JdbcPartitionStatisticStorage) factory.create(properties); @@ -696,10 +725,6 @@ protected void teardownStorage() throws Exception { /** Creates the partition_statistic_meta table in the PostgreSQL test database. */ private void createPostgreSQLSchema() throws SQLException { - String jdbcUrl = postgreSQLContainer.getJdbcUrl(TEST_DB_NAME); - String username = postgreSQLContainer.getUsername(); - String password = postgreSQLContainer.getPassword(); - try (Connection conn = DriverManager.getConnection(jdbcUrl, username, password); Statement stmt = conn.createStatement()) { @@ -728,6 +753,7 @@ private void createPostgreSQLSchema() throws SQLException { /** H2-specific tests using embedded in-memory database. */ @Nested + @CoreBackend.H2 static class H2Test extends BaseJdbcPartitionStatisticStorageTest { private static final String H2_JDBC_URL = diff --git a/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java b/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java index 896fe90ca42..de1fec6d173 100644 --- a/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java +++ b/core/src/test/java/org/apache/gravitino/storage/AbstractEntityStorageTest.java @@ -52,6 +52,7 @@ import java.sql.Statement; import java.time.Instant; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; @@ -78,8 +79,8 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.file.Fileset; -import org.apache.gravitino.integration.test.container.ContainerSuite; import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.CloseContainerExtension; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; @@ -95,21 +96,26 @@ import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.meta.UserEntity; import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.storage.relational.BackendTestSelector; +import org.apache.gravitino.storage.relational.CoreBackend; import org.apache.gravitino.storage.relational.RelationalBackend; import org.apache.gravitino.storage.relational.RelationalEntityStore; import org.apache.gravitino.storage.relational.RelationalGarbageCollector; +import org.apache.gravitino.storage.relational.SharedCoreDatabaseProvisioner; import org.apache.gravitino.storage.relational.converters.H2ExceptionConverter; import org.apache.gravitino.storage.relational.converters.MySQLExceptionConverter; import org.apache.gravitino.storage.relational.converters.PostgreSQLExceptionConverter; import org.apache.gravitino.storage.relational.converters.SQLExceptionConverterFactory; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.ibatis.session.SqlSession; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@CoreBackend.All +@ExtendWith(CloseContainerExtension.class) abstract class AbstractEntityStorageTest { protected static final Logger LOG = LoggerFactory.getLogger(AbstractEntityStorageTest.class); @@ -119,21 +125,18 @@ abstract class AbstractEntityStorageTest { protected static final String H2_FILE = DB_DIR + ".mv.db"; static Object[][] storageProvider() { - return new Object[][] { - {"h2", true}, - {"h2", false}, - {"mysql", true}, - {"mysql", false}, - {"postgresql", true}, - {"postgresql", false} - }; - } - - @AfterEach - void closeSuit() throws IOException { - // todo: refactor TestEntityStorage to extend TestJDBCBackend, otherwise, each test will start - // and stop the container suite. - ContainerSuite.getInstance().close(); + Object[][] backends = + new Object[][] { + {"h2", true}, + {"h2", false}, + {"mysql", true}, + {"mysql", false}, + {"postgresql", true}, + {"postgresql", false} + }; + return Arrays.stream(backends) + .filter(arguments -> BackendTestSelector.isSelected((String) arguments[0])) + .toArray(Object[][]::new); } protected void init(String type, Config config) throws IllegalAccessException { @@ -187,10 +190,16 @@ protected void init(String type, Config config) throws IllegalAccessException { SQLExceptionConverterFactory.class, "converter", new H2ExceptionConverter(), true); } else if (type.equalsIgnoreCase("mysql")) { - String mysqlJdbcUrl = baseIT.startAndInitMySQLBackend(); + boolean shared = SharedCoreDatabaseProvisioner.isEnabled("mysql"); + String mysqlJdbcUrl = + shared + ? SharedCoreDatabaseProvisioner.provision("mysql") + : baseIT.startAndInitMySQLBackend(); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(mysqlJdbcUrl); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.user("mysql") : "root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.password("mysql") : "root"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)) .thenReturn("com.mysql.cj.jdbc.Driver"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS)).thenReturn(100); @@ -201,10 +210,16 @@ protected void init(String type, Config config) throws IllegalAccessException { SQLExceptionConverterFactory.class, "converter", new MySQLExceptionConverter(), true); } else if (type.equalsIgnoreCase("postgresql")) { - String postgreSQLJdbcUrl = baseIT.startAndInitPGBackend(); + boolean shared = SharedCoreDatabaseProvisioner.isEnabled("postgresql"); + String postgreSQLJdbcUrl = + shared + ? SharedCoreDatabaseProvisioner.provision("postgresql") + : baseIT.startAndInitPGBackend(); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(postgreSQLJdbcUrl); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.user("postgresql") : "root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.password("postgresql") : "root"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)) .thenReturn("org.postgresql.Driver"); diff --git a/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java b/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java new file mode 100644 index 00000000000..7eb4760f571 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/TestBackendTestSelector.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; +import org.apache.gravitino.storage.relational.BackendTestSelector; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; + +/** Tests backend selection for the core database test suites. */ +@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ_WRITE) +public class TestBackendTestSelector { + + private static final String BACKEND_PROPERTY = "gravitino.core.test.backend"; + + private Optional originalBackend = Optional.empty(); + + @BeforeEach + void saveAndClearBackendProperty() { + originalBackend = Optional.ofNullable(System.getProperty(BACKEND_PROPERTY)); + System.clearProperty(BACKEND_PROPERTY); + } + + @AfterEach + void restoreBackendProperty() { + System.clearProperty(BACKEND_PROPERTY); + originalBackend.ifPresent(value -> System.setProperty(BACKEND_PROPERTY, value)); + } + + @Test + void testAbsentSelectionPreservesLegacyBehavior() { + assertEquals(Optional.empty(), BackendTestSelector.selectedBackend()); + assertTrue(BackendTestSelector.isSelected("h2")); + assertTrue(BackendTestSelector.isSelected("mysql")); + assertTrue(BackendTestSelector.isSelected("postgresql")); + } + + @Test + void testSelectionIsNormalizedAndValidated() { + System.setProperty(BACKEND_PROPERTY, " MySQL "); + + assertEquals(Optional.of("mysql"), BackendTestSelector.selectedBackend()); + assertTrue(BackendTestSelector.isSelected("MYSQL")); + assertFalse(BackendTestSelector.isSelected("h2")); + + System.setProperty(BACKEND_PROPERTY, "unsupported"); + assertThrows(IllegalArgumentException.class, BackendTestSelector::selectedBackend); + } + + @Test + void testStorageProviderUsesSelectedBackend() { + System.setProperty(BACKEND_PROPERTY, "postgresql"); + + assertArrayEquals( + new Object[][] {{"postgresql", true}, {"postgresql", false}}, + AbstractEntityStorageTest.storageProvider()); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java b/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java index e3ccdb44ccf..3affa9dca49 100644 --- a/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java +++ b/core/src/test/java/org/apache/gravitino/storage/TestSQLScripts.java @@ -38,6 +38,8 @@ import java.util.TreeSet; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.gravitino.storage.relational.DatabaseFixture; +import org.apache.gravitino.storage.relational.DatabaseIsolation; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; import org.apache.ibatis.session.SqlSession; @@ -45,6 +47,7 @@ import org.junit.jupiter.api.TestTemplate; import org.opentest4j.AssertionFailedError; +@DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) public class TestSQLScripts extends TestJDBCBackend { @TestTemplate diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java index 9961e30bb1c..a7532c0f797 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; @@ -50,12 +51,18 @@ import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.service.EntityIdService; import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.Extension; import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler; +import org.junit.jupiter.api.extension.ParameterContext; +import org.junit.jupiter.api.extension.ParameterResolutionException; +import org.junit.jupiter.api.extension.ParameterResolver; import org.junit.jupiter.api.extension.TestTemplateInvocationContext; import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider; +import org.junit.jupiter.api.extension.TestWatcher; import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,34 +77,44 @@ public class BackendTestExtension ExtensionContext.Namespace.create(BackendTestExtension.class); private static final String STORE_KEY = "BACKEND_MAP"; + private final BackendFactory backendFactory; + + /** Creates the extension with the production database-backend factory. */ + public BackendTestExtension() { + this(new DefaultBackendFactory()); + } + + BackendTestExtension(BackendFactory backendFactory) { + this.backendFactory = backendFactory; + } + @Override public void beforeAll(ExtensionContext context) { - // Initialize a Map and store it at the Class level. - // Key: backendType ("h2", "mysql"...), Value: RelationalBackend instance - context.getStore(NAMESPACE).put(STORE_KEY, new ConcurrentHashMap()); + context.getStore(NAMESPACE).put(STORE_KEY, new ConcurrentHashMap()); } @Override @SuppressWarnings("unchecked") - public void afterAll(ExtensionContext context) { - // Test class ended, close all started Backend - ConcurrentHashMap map = - (ConcurrentHashMap) context.getStore(NAMESPACE).get(STORE_KEY); + public void afterAll(ExtensionContext context) throws Exception { + ConcurrentHashMap map = + (ConcurrentHashMap) context.getStore(NAMESPACE).remove(STORE_KEY); if (map != null) { - map.forEach( - (type, backend) -> { - try { - LOG.info("Tearing down backend: {}", type); - backend.close(); - // H2 special cleaning logic - if ("h2".equals(type) && backend instanceof H2BackendWrapper) { - ((H2BackendWrapper) backend).cleanFile(); - } - } catch (Exception e) { - LOG.error("Failed to close backend {}", type, e); - } - }); + Exception failure = null; + for (BackendResource resource : map.values()) { + try { + resource.close(); + } catch (Exception e) { + if (failure == null) { + failure = e; + } else { + failure.addSuppressed(e); + } + } + } + if (failure != null) { + throw failure; + } } } @@ -109,6 +126,14 @@ public boolean supportsTestTemplate(ExtensionContext context) { @Override public Stream provideTestTemplateInvocationContexts( ExtensionContext context) { + String testMethodName = context.getRequiredTestMethod().getName(); + Optional selectedBackend = BackendTestSelector.selectedBackend(); + if (selectedBackend.isPresent()) { + LOG.info("Running tests with the selected {} backend.", selectedBackend.get()); + return createInvocationContexts( + testMethodName, Collections.singletonList(selectedBackend.get()), true); + } + List backendsToTest = new ArrayList<>(); backendsToTest.add("h2"); // Always test with H2 @@ -122,83 +147,111 @@ public Stream provideTestTemplateInvocationContex "Running tests with H2 backend only. Set env var 'dockerTest=true' to include all backends."); } - return backendsToTest.stream().map(BackendInvocationContext::new); + return createInvocationContexts(testMethodName, backendsToTest, false); + } + + Stream createInvocationContexts( + String testMethodName, List backends, boolean reuseBackend) { + return backends.stream() + .map( + backendType -> + new BackendInvocationContext( + testMethodName, backendType, reuseBackend, backendFactory)); } private static class BackendInvocationContext implements TestTemplateInvocationContext { + private final String testMethodName; private final String backendType; - - public BackendInvocationContext(String backendType) { + private final boolean reuseBackend; + private final BackendFactory backendFactory; + + private BackendInvocationContext( + String testMethodName, + String backendType, + boolean reuseBackend, + BackendFactory backendFactory) { + this.testMethodName = testMethodName; this.backendType = backendType; + this.reuseBackend = reuseBackend; + this.backendFactory = backendFactory; } @Override public String getDisplayName(int invocationIndex) { - return String.format("[%s Backend]", backendType.toUpperCase()); + // No trailing "()" here: @TestTemplate methods can declare parameters (e.g. an injected + // DatabaseTestContext), and a hardcoded empty parameter list would misrepresent the + // method's actual signature in JUnit XML/HTML reports. + return String.format("%s[%s Backend]", testMethodName, backendType.toUpperCase()); } @Override public List getAdditionalExtensions() { - return Collections.singletonList(new BackendSetupCallback(backendType)); + return Collections.singletonList( + new BackendSetupCallback(backendType, reuseBackend, backendFactory)); } } - private static class BackendSetupCallback implements BeforeEachCallback { - private final String backendType; - private final BaseIT baseIT = new BaseIT(); + @FunctionalInterface + interface BackendFactory { + BackendResource create(String backendType) throws Exception; + } - public BackendSetupCallback(String backendType) { - this.backendType = backendType; - } + @FunctionalInterface + interface BackendActivator { + void activate() throws Exception; + } + private static class DefaultBackendFactory implements BackendFactory { @Override - public void beforeEach(ExtensionContext context) throws Exception { - - // Lazy loading: If the backend of this type has not started, start it - RelationalBackend backend = startBackend(backendType); - - // Inject into the test instance - Object testInstance = context.getRequiredTestInstance(); - if (testInstance instanceof TestJDBCBackend) { - LOG.info("Injecting {} backend into test instance", backendType); - ((TestJDBCBackend) testInstance).setBackend(backend); - ((TestJDBCBackend) testInstance).setBackendType(backendType); - } - } - - private RelationalBackend startBackend(String type) throws Exception { - LOG.info("Initializing backend resource: {}", type); + public BackendResource create(String backendType) throws Exception { + BaseIT baseIT = new BaseIT(); + LOG.info("Initializing backend resource: {}", backendType); Config config = Mockito.mock(Config.class); Mockito.when(config.get(Configs.ENTITY_STORE)).thenReturn(Configs.RELATIONAL_ENTITY_STORE); Mockito.when(config.get(Configs.ENTITY_RELATIONAL_STORE)) .thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE); + // On the shared-container path, several test-JVM forks may hold a connection pool against + // the same physical server at once, so the Gradle 'core' test task publishes a smaller, + // shared-safe pool size (see SharedCoreDatabaseProvisioner.POOL_MAX_CONNECTIONS_PROPERTY) + // instead of the much larger per-fork-container default. Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS)) - .thenReturn(DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS); + .thenReturn( + Integer.getInteger( + SharedCoreDatabaseProvisioner.POOL_MAX_CONNECTIONS_PROPERTY, + DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_CONNECTIONS)); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_MAX_IDLE_CONNECTIONS)).thenReturn(10); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_WAIT_MILLISECONDS)) .thenReturn(DEFAULT_RELATIONAL_JDBC_BACKEND_MAX_WAIT_MILLISECONDS); Mockito.when(config.get(CACHE_ENABLED)).thenReturn(true); - - FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); - FieldUtils.writeField( - GravitinoEnv.getInstance(), "idGenerator", RandomIdGenerator.INSTANCE, true); - RelationalBackend backend = new JDBCBackend(); - if ("mysql".equals(type)) { - String url = baseIT.startAndInitMySQLBackend(); + if ("mysql".equals(backendType) || "postgresql".equals(backendType)) { + if (BackendTestSelector.selectedBackend().isPresent() + && !SharedCoreDatabaseProvisioner.isEnabled(backendType)) { + // The Gradle database tasks always publish the shared server. If it is missing, the + // build wiring broke; fail rather than quietly fall back to slow per-JVM containers. + throw new IllegalStateException( + "No shared " + + backendType + + " server was published to this test JVM; run it through the core " + + backendType + + " Gradle task."); + } + boolean shared = SharedCoreDatabaseProvisioner.isEnabled(backendType); + String url = + shared + ? SharedCoreDatabaseProvisioner.provision(backendType) + : "mysql".equals(backendType) + ? baseIT.startAndInitMySQLBackend() + : baseIT.startAndInitPGBackend(); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(url); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.user(backendType) : "root"); + Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)) + .thenReturn(shared ? SharedCoreDatabaseProvisioner.password(backendType) : "root"); Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)) - .thenReturn("com.mysql.cj.jdbc.Driver"); - } else if ("postgresql".equals(type)) { - String url = baseIT.startAndInitPGBackend(); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_URL)).thenReturn(url); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_USER)).thenReturn("root"); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD)).thenReturn("root"); - Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)) - .thenReturn("org.postgresql.Driver"); + .thenReturn( + "mysql".equals(backendType) ? "com.mysql.cj.jdbc.Driver" : "org.postgresql.Driver"); } else { // H2 Logic String uuid = UUID.randomUUID().toString().replace("-", ""); @@ -213,15 +266,255 @@ private RelationalBackend startBackend(String type) throws Exception { Mockito.when(config.get(ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER)).thenReturn("org.h2.Driver"); // Wrap it with a Wrapper so that files can be deleted during the clean process - backend = new H2BackendWrapper(config, jdbcPath); + backend = new H2BackendWrapper(jdbcPath); + } + + BackendResource resource = + new BackendResource( + backendType, + backend, + () -> { + FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); + FieldUtils.writeField( + GravitinoEnv.getInstance(), "idGenerator", RandomIdGenerator.INSTANCE, true); + EntityIdService.initialize(new RelationalEntityStoreIdResolver()); + }); + try { + resource.activate(); + // Close the backend before initializing to make sure the singleton sqlSession is cleared. + backend.close(); + backend.initialize(config); + return resource; + } catch (Exception failure) { + try { + resource.close(); + } catch (Exception closeFailure) { + failure.addSuppressed(closeFailure); + } + throw failure; + } + } + } + + private static class BackendSetupCallback + implements BeforeEachCallback, + AfterEachCallback, + LifecycleMethodExecutionExceptionHandler, + ParameterResolver, + TestWatcher { + private final String backendType; + private final boolean reuseBackend; + private final BackendFactory backendFactory; + private BackendResource backendResource; + private DatabaseTestContext databaseTestContext; + private boolean closeAfterEach; + + private BackendSetupCallback( + String backendType, boolean reuseBackend, BackendFactory backendFactory) { + this.backendType = backendType; + this.reuseBackend = reuseBackend; + this.backendFactory = backendFactory; + } + + @Override + public void beforeEach(ExtensionContext context) throws Exception { + DatabaseIsolation isolation = isolation(context); + if (isolation == DatabaseIsolation.DEDICATED_SERVER) { + throw new UnsupportedOperationException( + "DEDICATED_SERVER isolation is not implemented for core database tests"); + } + + closeAfterEach = !reuseBackend || isolation == DatabaseIsolation.FRESH_NAMESPACE; + try { + if (reuseBackend && isolation == DatabaseIsolation.FRESH_NAMESPACE) { + closeClassBackend(context); + } + backendResource = closeAfterEach ? newBackendResource() : getOrCreateClassBackend(context); + backendResource.activate(); + databaseTestContext = + new DatabaseTestContext(backendType, backendResource.backend, isolation); + } catch (Exception e) { + cleanupAfterSetupFailure(e); + throw e; + } + } + + @Override + public boolean supportsParameter( + ParameterContext parameterContext, ExtensionContext extensionContext) { + return parameterContext.getParameter().getType() == DatabaseTestContext.class; + } + + @Override + public DatabaseTestContext resolveParameter( + ParameterContext parameterContext, ExtensionContext extensionContext) { + if (databaseTestContext == null) { + throw new ParameterResolutionException( + "DatabaseTestContext is unavailable before the database fixture starts"); + } + return databaseTestContext; + } + + @Override + public void afterEach(ExtensionContext context) throws Exception { + try { + if (closeAfterEach && backendResource != null) { + backendResource.close(); + backendResource = null; + } + } finally { + databaseTestContext = null; + } + } + + @Override + public void testAborted(ExtensionContext context, Throwable cause) { + poisonSharedFixture(); + } + + @Override + public void testFailed(ExtensionContext context, Throwable cause) { + poisonSharedFixture(); + } + + @Override + public void handleBeforeEachMethodExecutionException( + ExtensionContext context, Throwable throwable) throws Throwable { + poisonSharedFixture(); + throw throwable; + } + + @Override + public void handleAfterEachMethodExecutionException( + ExtensionContext context, Throwable throwable) throws Throwable { + poisonSharedFixture(); + throw throwable; + } + + private BackendResource getOrCreateClassBackend(ExtensionContext context) throws Exception { + ConcurrentHashMap backendMap = backendMap(context); + synchronized (backendMap) { + BackendResource resource = backendMap.get(backendType); + if (resource != null && resource.poisoned) { + backendMap.remove(backendType); + resource.close(); + resource = null; + } + if (resource == null) { + resource = newBackendResource(); + backendMap.put(backendType, resource); + } + return resource; } + } - // close the backend before initializing to make sure the singleton sqlSession - // has been cleared. - backend.close(); - backend.initialize(config); - EntityIdService.initialize(new RelationalEntityStoreIdResolver()); - return backend; + private void closeClassBackend(ExtensionContext context) throws Exception { + ConcurrentHashMap backendMap = backendMap(context); + synchronized (backendMap) { + BackendResource resource = backendMap.remove(backendType); + if (resource != null) { + resource.close(); + } + } + } + + @SuppressWarnings("unchecked") + private ConcurrentHashMap backendMap(ExtensionContext context) { + ExtensionContext classContext = context; + while (classContext.getTestMethod().isPresent()) { + classContext = + classContext + .getParent() + .orElseThrow(() -> new IllegalStateException("Test class context is unavailable")); + } + + ConcurrentHashMap backendMap = + (ConcurrentHashMap) + classContext.getStore(NAMESPACE).get(STORE_KEY); + if (backendMap == null) { + throw new IllegalStateException("Backend fixture store is unavailable"); + } + return backendMap; + } + + private BackendResource newBackendResource() throws Exception { + return backendFactory.create(backendType); + } + + private void cleanupAfterSetupFailure(Exception failure) { + databaseTestContext = null; + if (backendResource == null) { + return; + } + if (!closeAfterEach) { + backendResource.poisoned = true; + return; + } + try { + backendResource.close(); + } catch (Exception closeFailure) { + failure.addSuppressed(closeFailure); + } finally { + backendResource = null; + } + } + + private void poisonSharedFixture() { + if (!closeAfterEach && backendResource != null) { + backendResource.poisoned = true; + } + } + + private DatabaseIsolation isolation(ExtensionContext context) { + DatabaseFixture methodFixture = + context.getRequiredTestMethod().getAnnotation(DatabaseFixture.class); + if (methodFixture != null) { + return methodFixture.value(); + } + DatabaseFixture classFixture = + context.getRequiredTestClass().getAnnotation(DatabaseFixture.class); + return classFixture == null ? DatabaseIsolation.RESETTABLE_NAMESPACE : classFixture.value(); + } + } + + static class BackendResource { + private final String backendType; + private final RelationalBackend backend; + private final BackendActivator activator; + private boolean poisoned; + + BackendResource(String backendType, RelationalBackend backend, BackendActivator activator) { + this.backendType = backendType; + this.backend = backend; + this.activator = activator; + } + + private void activate() throws Exception { + activator.activate(); + } + + private void close() throws Exception { + LOG.info("Tearing down backend: {}", backendType); + Exception failure = null; + try { + backend.close(); + } catch (Exception e) { + failure = e; + } + try { + if (backend instanceof H2BackendWrapper) { + ((H2BackendWrapper) backend).cleanFile(); + } + } catch (Exception e) { + if (failure == null) { + failure = e; + } else { + failure.addSuppressed(e); + } + } + if (failure != null) { + throw failure; + } } } @@ -229,9 +522,8 @@ private RelationalBackend startBackend(String type) throws Exception { public static class H2BackendWrapper extends JDBCBackend { private final String path; - public H2BackendWrapper(Config config, String path) { + public H2BackendWrapper(String path) { this.path = path; - super.initialize(config); } public void cleanFile() throws IOException { diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java new file mode 100644 index 00000000000..3f794dbea80 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/BackendTestSelector.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational; + +import java.util.Locale; +import java.util.Optional; +import java.util.Set; + +/** Selects a single JDBC backend for the core database test suites. */ +public final class BackendTestSelector { + + private static final String BACKEND_PROPERTY = "gravitino.core.test.backend"; + private static final Set SUPPORTED_BACKENDS = Set.of("h2", "mysql", "postgresql"); + + private BackendTestSelector() {} + + /** + * Returns the selected backend, or an empty value when the legacy all-applicable-backends + * behavior should be used. + * + * @return the normalized selected backend + * @throws IllegalArgumentException if the configured backend is unsupported + */ + public static Optional selectedBackend() { + String configuredBackend = System.getProperty(BACKEND_PROPERTY); + if (configuredBackend == null) { + return Optional.empty(); + } + + return Optional.of(validate(configuredBackend)); + } + + /** + * Returns whether a backend should run under the current selection. + * + * @param backend backend to test + * @return true when no backend is selected or the backend matches the selection + * @throws IllegalArgumentException if either backend value is unsupported + */ + public static boolean isSelected(String backend) { + String normalizedBackend = validate(backend); + return selectedBackend().map(normalizedBackend::equals).orElse(true); + } + + private static String validate(String backend) { + String normalizedBackend = backend.trim().toLowerCase(Locale.ROOT); + if (!SUPPORTED_BACKENDS.contains(normalizedBackend)) { + throw new IllegalArgumentException( + String.format( + "Unsupported core test backend '%s'; expected one of %s", + backend, SUPPORTED_BACKENDS)); + } + return normalizedBackend; + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java new file mode 100644 index 00000000000..65d2fd47865 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/CoreBackend.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.junit.jupiter.api.Tag; + +/** + * Annotations that choose which core database backends a class runs against. {@code test} runs + * classes with no backend tag and classes tagged for H2; {@code coreMySQLTest} and {@code + * corePostgreSQLTest} run only classes tagged for their backend. Stack annotations to join several + * backends, or use {@link All}. {@code TestCoreLaneMembership} fails if a class carries {@code + * gravitino-docker-test} but no backend tag, since it would run in no task. + * + *

The lane, not the class, decides whether Docker is needed: MySQL and PostgreSQL require it and + * H2 does not, so a class tagged for H2 must not need a container itself. The tag strings must stay + * in sync with {@code coreBackendTestTags} in {@code core/build.gradle.kts}. + */ +public final class CoreBackend { + + /** The JUnit tag that selects the H2 lane. */ + public static final String H2_TAG = "gravitino-core-h2-test"; + + /** The JUnit tag that selects the MySQL lane. */ + public static final String MYSQL_TAG = "gravitino-core-mysql-test"; + + /** The JUnit tag that selects the PostgreSQL lane. */ + public static final String POSTGRESQL_TAG = "gravitino-core-postgresql-test"; + + private CoreBackend() {} + + /** Runs a core database test class against H2, in {@code test}. */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(H2_TAG) + public @interface H2 {} + + /** Pins a core database test class to the MySQL lane ({@code coreMySQLTest}). */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(MYSQL_TAG) + public @interface MySQL {} + + /** Pins a core database test class to the PostgreSQL lane ({@code corePostgreSQLTest}). */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(POSTGRESQL_TAG) + public @interface PostgreSQL {} + + /** Runs a core database test class under every backend lane. */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + @Tag(H2_TAG) + @Tag(MYSQL_TAG) + @Tag(POSTGRESQL_TAG) + public @interface All {} +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java new file mode 100644 index 00000000000..6c966af4b0c --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseFixture.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** Declares the database-fixture isolation required by a test class or method. */ +@Documented +@Inherited +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.TYPE, ElementType.METHOD}) +public @interface DatabaseFixture { + + /** + * Returns the required database isolation. + * + * @return the isolation policy + */ + DatabaseIsolation value() default DatabaseIsolation.RESETTABLE_NAMESPACE; +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java new file mode 100644 index 00000000000..c87d27ca6ca --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseIsolation.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; + +/** Isolation policies for database-backed tests. */ +public enum DatabaseIsolation { + /** Reuse a class-scoped fixture and reset its tables before each test. */ + RESETTABLE_NAMESPACE, + + /** Create and close a fresh database backend for each test. */ + FRESH_NAMESPACE, + + /** Create a dedicated database server for each test. */ + DEDICATED_SERVER +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseTestContext.java b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseTestContext.java new file mode 100644 index 00000000000..6b8435dcf33 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/DatabaseTestContext.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; + +import java.util.Objects; + +/** + * Provides the database fixture owned by the current test invocation. + * + *

The extension owns the fixture lifecycle. Tests must not close the supplied backend. + */ +public final class DatabaseTestContext { + private final String backendType; + private final RelationalBackend backend; + private final DatabaseIsolation isolation; + + DatabaseTestContext(String backendType, RelationalBackend backend, DatabaseIsolation isolation) { + this.backendType = Objects.requireNonNull(backendType, "backendType"); + this.backend = Objects.requireNonNull(backend, "backend"); + this.isolation = Objects.requireNonNull(isolation, "isolation"); + } + + /** Returns the database implementation selected for this invocation. */ + public String backendType() { + return backendType; + } + + /** Returns the initialized relational backend owned by the test fixture. */ + public RelationalBackend backend() { + return backend; + } + + /** Returns the isolation policy requested by the test. */ + public DatabaseIsolation isolation() { + return isolation; + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/SharedCoreDatabaseProvisioner.java b/core/src/test/java/org/apache/gravitino/storage/relational/SharedCoreDatabaseProvisioner.java new file mode 100644 index 00000000000..a3fd768f151 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/SharedCoreDatabaseProvisioner.java @@ -0,0 +1,235 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Arrays; +import java.util.Objects; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nullable; +import org.apache.gravitino.config.ConfigConstants; +import org.apache.gravitino.integration.test.container.DatabaseProvisioning; + +/** + * Provisions an isolated database for one Core database test-JVM fork on a shared MySQL/PostgreSQL + * server, instead of that fork starting its own container. The server is either one injected by the + * environment ({@code GRAVITINO_TEST_MYSQL_URL} and friends, see {@code core/build.gradle.kts}) or + * the build-wide container that {@code SharedDbContainerService} (buildSrc) starts. + * + *

Active only when the Gradle task has published admin connection info as system properties + * under {@link #ADMIN_PROPERTY_PREFIX}; {@link #isEnabled(String)} returns {@code false} otherwise, + * e.g. when this test class is run directly from an IDE outside the Gradle task, and the caller is + * expected to fall back to starting a fresh, per-fork container as before. + * + *

Each call to {@link #provision(String)} allocates and migrates a brand-new database - this + * relies on {@link BackendTestExtension} only calling it when a genuinely new backend is actually + * needed (a fresh {@link DatabaseTestContext} fixture, or the first fixture for a test class); + * reuse across test methods within a class is already handled one layer up, by {@link + * BackendTestExtension}'s own class-scoped backend cache. Databases provisioned here are never + * individually dropped; they are discarded together with the shared container when the build ends + * (see {@code SharedDbContainerService#close()} in buildSrc). + */ +public final class SharedCoreDatabaseProvisioner { + + /** System property prefix under which the shared-container BuildService publishes info. */ + static final String ADMIN_PROPERTY_PREFIX = "gravitino.test.db."; + + /** System property carrying the shared-safe connection-pool budget, if published. */ + static final String POOL_MAX_CONNECTIONS_PROPERTY = ADMIN_PROPERTY_PREFIX + "pool.maxConnections"; + + private static final AtomicLong DATABASE_COUNTER = new AtomicLong(); + + // Differs per test JVM, so a later run on a long-lived injected server never reuses an earlier + // run's database (and its rows) under the same name. + private static final String RUN_TOKEN = + Long.toHexString(ThreadLocalRandom.current().nextLong() & 0xffffffffL); + + private SharedCoreDatabaseProvisioner() {} + + /** + * Returns whether the shared-container BuildService has published admin connection info for + * {@code backendType} ({@code "mysql"} or {@code "postgresql"}). + */ + public static boolean isEnabled(String backendType) { + return adminUrl(backendType) != null; + } + + /** + * Creates and migrates a brand-new, uniquely-named database on the shared container for {@code + * backendType}, returning its JDBC URL. + * + * @throws IllegalStateException if {@link #isEnabled(String)} is {@code false} for {@code + * backendType} + */ + public static String provision(String backendType) throws IOException { + String dbUrl = createDatabase(backendType); + runSchemaMigration(backendType, dbUrl, user(backendType), password(backendType)); + return dbUrl; + } + + /** + * Creates a brand-new, empty, uniquely-named database on the shared server for {@code + * backendType} and returns its JDBC URL, for tests that create their own tables. + * + * @throws IllegalStateException if {@link #isEnabled(String)} is {@code false} for {@code + * backendType} + */ + public static String createDatabase(String backendType) { + String adminUrl = adminUrl(backendType); + if (adminUrl == null) { + throw new IllegalStateException("No shared database server is set for " + backendType); + } + if ("mysql".equals(backendType)) { + loadMySQLDriver(); + } + String dbName = nextDatabaseName(); + DatabaseProvisioning.createDatabaseIfAbsent( + adminUrl, user(backendType), password(backendType), dbName); + return buildJdbcUrlForDatabase(adminUrl, dbName); + } + + /** The user for databases on the shared server for {@code backendType}. */ + public static String user(String backendType) { + return System.getProperty(ADMIN_PROPERTY_PREFIX + backendType + ".user"); + } + + /** The password for databases on the shared server for {@code backendType}. */ + public static String password(String backendType) { + return System.getProperty(ADMIN_PROPERTY_PREFIX + backendType + ".password"); + } + + /** + * Forces the legacy MySQL JDBC driver class to load before the first {@link + * DriverManager#getConnection}, mirroring the same workaround {@code + * MySQLContainer#createDatabase} already applies for #6392 (the driver may not register + * itself via {@link java.util.ServiceLoader} in time on some JVMs/classloaders). + */ + private static void loadMySQLDriver() { + try { + Class.forName("com.mysql.jdbc.Driver"); + } catch (ClassNotFoundException e) { + throw new RuntimeException("Failed to load MySQL JDBC driver", e); + } + } + + @Nullable + private static String adminUrl(String backendType) { + String value = System.getProperty(ADMIN_PROPERTY_PREFIX + backendType + ".adminUrl"); + return (value == null || value.isEmpty()) ? null : value; + } + + /** + * Computes a fresh database name: {@code "gravitino_core_" + token + "_" + sanitize(worker) + "_" + * + counter}, where {@code token} is random per JVM, where {@code worker} is {@code + * System.getProperty("org.gradle.test.worker", "local")} and {@code counter} is a per-JVM + * monotonically increasing value - unlike a name derived only from a test class name, this can + * never collide within one fork no matter how many databases that fork ends up provisioning, and + * the worker prefix keeps distinct forks apart from each other. + */ + static String nextDatabaseName() { + String worker = + System.getProperty("org.gradle.test.worker", "local").replaceAll("[^a-zA-Z0-9_$]", "_"); + String name = + "gravitino_core_" + RUN_TOKEN + "_" + worker + "_" + DATABASE_COUNTER.incrementAndGet(); + if (!DatabaseProvisioning.isValidDatabaseName(name)) { + // Only reachable for an absurdly long worker id - fail loudly instead of silently + // colliding or truncating into an unrelated name. + throw new IllegalStateException("Computed an invalid shared database name: " + name); + } + return name; + } + + /** + * Derives the JDBC URL to connect directly to database {@code dbName} on the same server as + * {@code adminUrl}, by replacing whatever database name (if any) {@code adminUrl} points at. + */ + static String buildJdbcUrlForDatabase(String adminUrl, String dbName) { + int queryIndex = adminUrl.indexOf('?'); + String base = queryIndex >= 0 ? adminUrl.substring(0, queryIndex) : adminUrl; + String query = queryIndex >= 0 ? adminUrl.substring(queryIndex) : ""; + + int schemeEnd = base.indexOf("://"); + int hostStart = schemeEnd >= 0 ? schemeEnd + 3 : 0; + int pathStart = base.indexOf('/', hostStart); + String hostPart = pathStart >= 0 ? base.substring(0, pathStart) : base; + return hostPart + "/" + dbName + query; + } + + /** + * Runs the same OSS schema SQL that {@link + * org.apache.gravitino.integration.test.util.BaseIT#startAndInitMySQLBackend()} / {@link + * org.apache.gravitino.integration.test.util.BaseIT#startAndInitPGBackend()} apply, against the + * given already-created, empty database. + */ + private static void runSchemaMigration(String type, String dbUrl, String user, String password) + throws IOException { + String gravitinoHome = + Objects.requireNonNull( + System.getenv("GRAVITINO_ROOT_DIR"), + "GRAVITINO_ROOT_DIR must be set to locate the schema SQL to migrate a shared " + + type + + " database"); + String schemaContent = loadSchemaContent(gravitinoHome, type); + + String[] statements = + Arrays.stream(schemaContent.split(";")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .toArray(String[]::new); + + String currentStatement = ""; + try (Connection connection = DriverManager.getConnection(dbUrl, user, password); + Statement statement = connection.createStatement()) { + for (String sql : statements) { + currentStatement = sql; + statement.execute(sql); + } + } catch (SQLException e) { + throw new RuntimeException( + "Failed to migrate shared " + + type + + " database " + + dbUrl + + ", statement: " + + currentStatement, + e); + } + } + + private static String loadSchemaContent(String gravitinoHome, String databaseType) + throws IOException { + String version = ConfigConstants.CURRENT_SCRIPT_VERSION; + Path schema = + Path.of( + gravitinoHome, + "scripts", + databaseType, + String.format("schema-%s-%s.sql", version, databaseType)); + return Files.readString(schema, StandardCharsets.UTF_8); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java new file mode 100644 index 00000000000..47b878e7e35 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestBackendTestExtension.java @@ -0,0 +1,427 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.Extension; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler; +import org.junit.jupiter.api.extension.ParameterContext; +import org.junit.jupiter.api.extension.ParameterResolutionException; +import org.junit.jupiter.api.extension.ParameterResolver; +import org.junit.jupiter.api.extension.TestTemplateInvocationContext; +import org.junit.jupiter.api.extension.TestWatcher; + +class TestBackendTestExtension { + + @Test + void testSelectedBackendReusesClassFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + ExtensionContext firstContext = contexts.methodContext("first"); + Extension firstCallback = startInvocation(extension, true, firstContext); + DatabaseTestContext databaseContext = + (DatabaseTestContext) + ((ParameterResolver) firstCallback).resolveParameter(parameterContext(0), firstContext); + assertEquals("h2", databaseContext.backendType()); + assertSame(factory.backends.get(0), databaseContext.backend()); + assertEquals(DatabaseIsolation.RESETTABLE_NAMESPACE, databaseContext.isolation()); + ((AfterEachCallback) firstCallback).afterEach(firstContext); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(1, factory.backends.size()); + assertEquals(2, factory.activationCount); + verify(factory.backends.get(0), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(0), times(1)).close(); + } + + @Test + void testLegacyH2DoesNotReuseFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, false, contexts.methodContext("first")); + runInvocation(extension, false, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + + extension.afterAll(contexts.classContext); + } + + @Test + void testFreshClassCreatesAndClosesEachInvocation() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(FreshIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("first")); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + + extension.afterAll(contexts.classContext); + } + + @Test + void testFreshMethodEvictsSharedFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(MixedIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("resettable")); + runInvocation(extension, true, contexts.methodContext("fresh")); + runInvocation(extension, true, contexts.methodContext("resettable")); + + assertEquals(3, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + verify(factory.backends.get(2), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(2), times(1)).close(); + } + + @Test + void testFailedSharedFixtureIsClosedAndRebuilt() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + ExtensionContext firstContext = contexts.methodContext("first"); + Extension firstCallback = startInvocation(extension, true, firstContext); + ((AfterEachCallback) firstCallback).afterEach(firstContext); + ((TestWatcher) firstCallback).testFailed(firstContext, new AssertionError("expected")); + + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), never()).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testLifecycleFailurePoisonsSharedFixture() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + ExtensionContext firstContext = contexts.methodContext("first"); + Extension firstCallback = startInvocation(extension, true, firstContext); + assertThrows( + IllegalStateException.class, + () -> + ((LifecycleMethodExecutionExceptionHandler) firstCallback) + .handleBeforeEachMethodExecutionException( + firstContext, new IllegalStateException("expected"))); + + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, factory.backends.size()); + verify(factory.backends.get(0), times(1)).close(); + + extension.afterAll(contexts.classContext); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testActivationFailurePoisonsSharedFixture() throws Exception { + List backends = new ArrayList<>(); + AtomicInteger activations = new AtomicInteger(); + BackendTestExtension.BackendFactory factory = + backendType -> { + RelationalBackend backend = mock(RelationalBackend.class); + backends.add(backend); + return new BackendTestExtension.BackendResource( + backendType, + backend, + () -> { + if (activations.incrementAndGet() == 2) { + throw new IllegalStateException("expected"); + } + }); + }; + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, contexts.methodContext("first")); + assertThrows( + IllegalStateException.class, + () -> startInvocation(extension, true, contexts.methodContext("second"))); + runInvocation(extension, true, contexts.methodContext("second")); + + assertEquals(2, backends.size()); + verify(backends.get(0), times(1)).close(); + + extension.afterAll(contexts.classContext); + verify(backends.get(1), times(1)).close(); + } + + @Test + void testDedicatedServerIsolationFailsFast() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DedicatedServerTests.class); + extension.beforeAll(contexts.classContext); + + assertThrows( + UnsupportedOperationException.class, + () -> startInvocation(extension, true, contexts.methodContext("first"))); + assertEquals(0, factory.backends.size()); + + extension.afterAll(contexts.classContext); + } + + @Test + void testAfterAllClosesEveryFixtureAndAggregatesFailures() throws Exception { + CountingBackendFactory factory = new CountingBackendFactory(); + BackendTestExtension extension = new BackendTestExtension(factory); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + + runInvocation(extension, true, "h2", contexts.methodContext("first")); + runInvocation(extension, true, "mysql", contexts.methodContext("second")); + doThrow(new IOException("h2 close")).when(factory.backends.get(0)).close(); + doThrow(new IOException("mysql close")).when(factory.backends.get(1)).close(); + + Exception failure = + assertThrows(Exception.class, () -> extension.afterAll(contexts.classContext)); + assertEquals(1, failure.getSuppressed().length); + verify(factory.backends.get(0), times(1)).close(); + verify(factory.backends.get(1), times(1)).close(); + } + + @Test + void testH2CleanupRunsAfterUncheckedBackendCloseFailure() throws Exception { + BackendTestExtension.H2BackendWrapper backend = + mock(BackendTestExtension.H2BackendWrapper.class); + BackendTestExtension extension = + new BackendTestExtension( + backendType -> + new BackendTestExtension.BackendResource(backendType, backend, () -> {})); + TestContexts contexts = new TestContexts(DefaultIsolationTests.class); + extension.beforeAll(contexts.classContext); + runInvocation(extension, true, contexts.methodContext("first")); + + doThrow(new IllegalStateException("close")).when(backend).close(); + doThrow(new IOException("clean")).when(backend).cleanFile(); + + Exception failure = + assertThrows(Exception.class, () -> extension.afterAll(contexts.classContext)); + assertEquals("close", failure.getMessage()); + assertEquals(1, failure.getSuppressed().length); + assertEquals("clean", failure.getSuppressed()[0].getMessage()); + verify(backend, times(1)).cleanFile(); + } + + @Test + void testDisplayNamePreservesMethodAndBackend() { + BackendTestExtension extension = + new BackendTestExtension( + backendType -> + new BackendTestExtension.BackendResource( + backendType, mock(RelationalBackend.class), () -> {})); + + TestTemplateInvocationContext context = + extension + .createInvocationContexts("testMethod", List.of("h2"), true) + .findFirst() + .orElseThrow(); + ParameterResolver resolver = (ParameterResolver) context.getAdditionalExtensions().get(0); + + assertEquals("testMethod[H2 Backend]", context.getDisplayName(1)); + assertTrue(resolver.supportsParameter(parameterContext(0), mock(ExtensionContext.class))); + assertFalse(resolver.supportsParameter(parameterContext(1), mock(ExtensionContext.class))); + assertThrows( + ParameterResolutionException.class, + () -> resolver.resolveParameter(parameterContext(0), mock(ExtensionContext.class))); + } + + private static void runInvocation( + BackendTestExtension extension, boolean reuseBackend, ExtensionContext context) + throws Exception { + runInvocation(extension, reuseBackend, "h2", context); + } + + private static void runInvocation( + BackendTestExtension extension, + boolean reuseBackend, + String backendType, + ExtensionContext context) + throws Exception { + Extension callback = startInvocation(extension, reuseBackend, backendType, context); + ((AfterEachCallback) callback).afterEach(context); + } + + private static Extension startInvocation( + BackendTestExtension extension, boolean reuseBackend, ExtensionContext context) + throws Exception { + return startInvocation(extension, reuseBackend, "h2", context); + } + + private static Extension startInvocation( + BackendTestExtension extension, + boolean reuseBackend, + String backendType, + ExtensionContext context) + throws Exception { + TestTemplateInvocationContext invocation = + extension + .createInvocationContexts("testMethod", List.of(backendType), reuseBackend) + .findFirst() + .orElseThrow(); + Extension callback = invocation.getAdditionalExtensions().get(0); + ((BeforeEachCallback) callback).beforeEach(context); + return callback; + } + + private static ParameterContext parameterContext(int index) { + Parameter parameter; + try { + parameter = + index == 0 + ? TestJDBCBackend.class.getDeclaredMethod("init", DatabaseTestContext.class) + .getParameters()[0] + : Object.class.getDeclaredMethod("equals", Object.class).getParameters()[0]; + } catch (NoSuchMethodException e) { + throw new AssertionError(e); + } + ParameterContext context = mock(ParameterContext.class); + when(context.getParameter()).thenReturn(parameter); + return context; + } + + private static class CountingBackendFactory implements BackendTestExtension.BackendFactory { + private final List backends = new ArrayList<>(); + private int activationCount; + + @Override + public BackendTestExtension.BackendResource create(String backendType) { + RelationalBackend backend = mock(RelationalBackend.class); + backends.add(backend); + return new BackendTestExtension.BackendResource( + backendType, backend, () -> activationCount++); + } + } + + private static class TestContexts { + private final Class testClass; + private final ExtensionContext classContext; + + private TestContexts(Class testClass) { + this.testClass = testClass; + classContext = mock(ExtensionContext.class); + ExtensionContext.Store store = mock(ExtensionContext.Store.class); + Map entries = new HashMap<>(); + + doAnswer( + invocation -> { + entries.put(invocation.getArgument(0), invocation.getArgument(1)); + return null; + }) + .when(store) + .put(any(), any()); + when(store.get(any())).thenAnswer(invocation -> entries.get(invocation.getArgument(0))); + when(store.remove(any())).thenAnswer(invocation -> entries.remove(invocation.getArgument(0))); + when(classContext.getStore(any(ExtensionContext.Namespace.class))).thenReturn(store); + when(classContext.getTestMethod()).thenReturn(Optional.empty()); + when(classContext.getRequiredTestClass()).thenAnswer(invocation -> testClass); + } + + private ExtensionContext methodContext(String methodName) throws NoSuchMethodException { + Method method = testClass.getDeclaredMethod(methodName); + ExtensionContext methodContext = mock(ExtensionContext.class); + when(methodContext.getTestMethod()).thenReturn(Optional.of(method)); + when(methodContext.getRequiredTestMethod()).thenReturn(method); + when(methodContext.getRequiredTestClass()).thenAnswer(invocation -> testClass); + when(methodContext.getRequiredTestInstance()).thenReturn(this); + when(methodContext.getParent()).thenReturn(Optional.of(classContext)); + return methodContext; + } + } + + private static class DefaultIsolationTests { + void first() {} + + void second() {} + } + + @DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) + private static class FreshIsolationTests { + void first() {} + + void second() {} + } + + private static class MixedIsolationTests { + void resettable() {} + + @DatabaseFixture(DatabaseIsolation.FRESH_NAMESPACE) + void fresh() {} + } + + @DatabaseFixture(DatabaseIsolation.DEDICATED_SERVER) + private static class DedicatedServerTests { + void first() {} + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreLaneMembership.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreLaneMembership.java new file mode 100644 index 00000000000..b04c8960df5 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestCoreLaneMembership.java @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Modifier; +import java.net.URL; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.platform.commons.annotation.Testable; +import org.junit.platform.commons.support.AnnotationSupport; +import org.junit.platform.commons.support.HierarchyTraversalMode; +import org.junit.platform.commons.support.ReflectionSupport; + +/** + * Fails when a core test class runs in no task: it carries {@code gravitino-docker-test}, which + * keeps it out of {@code test}, but no {@link CoreBackend} tag, which keeps it out of the database + * tasks. Nothing else notices a class that leaves every task at once. + */ +class TestCoreLaneMembership { + + private static final String DOCKER_TAG = "gravitino-docker-test"; + + private static final Set BACKEND_TAGS = + Set.of(CoreBackend.H2_TAG, CoreBackend.MYSQL_TAG, CoreBackend.POSTGRESQL_TAG); + + @Test + void everyDockerTaggedTestClassHasABackendLane() { + URL coreTestClasses = codeLocation(TestCoreLaneMembership.class); + List orphans = + ReflectionSupport.findAllClassesInPackage( + "org.apache.gravitino", + c -> coreTestClasses.equals(codeLocation(c)) && declaresTests(c), + name -> true) + .stream() + .filter( + c -> { + Set tags = effectiveTags(c); + return tags.contains(DOCKER_TAG) + && tags.stream().noneMatch(BACKEND_TAGS::contains); + }) + .map(Class::getName) + .sorted() + .collect(Collectors.toList()); + + assertTrue( + orphans.isEmpty(), + "These classes carry gravitino-docker-test but no @CoreBackend.* tag, so they run in no " + + "core test lane. Add @CoreBackend.H2/MySQL/PostgreSQL or @CoreBackend.All: " + + orphans); + } + + /** + * Whether JUnit would run {@code c}: concrete, with at least one {@code @Test}-style method (all + * meta-annotated with {@link Testable}), own or inherited. + */ + private static boolean declaresTests(Class c) { + return !Modifier.isAbstract(c.getModifiers()) + && !ReflectionSupport.findMethods( + c, + m -> AnnotationSupport.isAnnotated(m, Testable.class), + HierarchyTraversalMode.TOP_DOWN) + .isEmpty(); + } + + /** + * Own and inherited tags, plus the enclosing classes' tags for non-static {@code @Nested} + * classes, which JUnit applies to them. Static member classes are discovered as top-level classes + * and don't inherit their enclosing class's tags. + */ + private static Set effectiveTags(Class c) { + Set tags = new HashSet<>(); + for (Class k = c; + k != null; + k = Modifier.isStatic(k.getModifiers()) ? null : k.getEnclosingClass()) { + AnnotationSupport.findRepeatableAnnotations(k, Tag.class).forEach(t -> tags.add(t.value())); + } + return tags; + } + + private static URL codeLocation(Class c) { + return c.getProtectionDomain().getCodeSource() == null + ? null + : c.getProtectionDomain().getCodeSource().getLocation(); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java index 5c878525c27..ea1191cedf6 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackend.java @@ -84,6 +84,7 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtendWith; +@CoreBackend.All @TestInstance(TestInstance.Lifecycle.PER_CLASS) @ExtendWith({ BackendTestExtension.class, @@ -97,16 +98,16 @@ public abstract class TestJDBCBackend { protected String backendType; protected RelationalBackend backend; - public void setBackend(RelationalBackend backend) { - this.backend = backend; - } - - public void setBackendType(String backendType) { - this.backendType = backendType; + /** Activates and resets the database fixture selected for the current test invocation. */ + @BeforeEach + public void init(DatabaseTestContext context) throws SQLException { + backend = context.backend(); + backendType = context.backendType(); + resetDatabase(); } - @BeforeEach - public void init() throws SQLException { + /** Clears data owned by the current database fixture. */ + protected void resetDatabase() throws SQLException { truncateAllTables(); } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/TestSharedCoreDatabaseProvisioner.java b/core/src/test/java/org/apache/gravitino/storage/relational/TestSharedCoreDatabaseProvisioner.java new file mode 100644 index 00000000000..329b1d013f3 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/TestSharedCoreDatabaseProvisioner.java @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import javax.annotation.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link SharedCoreDatabaseProvisioner}'s pure logic: {@link + * SharedCoreDatabaseProvisioner#isEnabled(String)}, {@link + * SharedCoreDatabaseProvisioner#nextDatabaseName()}, and {@link + * SharedCoreDatabaseProvisioner#buildJdbcUrlForDatabase(String, String)}. Provisioning against a + * real server is exercised indirectly by the docker-tagged integration tests instead. + */ +public class TestSharedCoreDatabaseProvisioner { + + private static final String ADMIN_URL_PROPERTY = + SharedCoreDatabaseProvisioner.ADMIN_PROPERTY_PREFIX + "mysql.adminUrl"; + + // org.gradle.test.worker is set by Gradle itself for the lifetime of a forked test-worker JVM + // and is also read elsewhere in the same JVM (CoreDatabaseShardTelemetryExtension), so tests + // must save and restore it exactly as they found it rather than clearing it outright - Gradle + // reuses worker JVMs across test classes, so clearing it here would corrupt telemetry (and, + // for any future database-tagged class sharing this fork, database naming) for every class + // that happens to run afterward in the same fork. + private @Nullable String originalAdminUrlProperty; + private @Nullable String originalWorkerProperty; + + @BeforeEach + public void saveProperties() { + originalAdminUrlProperty = System.getProperty(ADMIN_URL_PROPERTY); + originalWorkerProperty = System.getProperty("org.gradle.test.worker"); + } + + @AfterEach + public void restoreProperties() { + restoreProperty(ADMIN_URL_PROPERTY, originalAdminUrlProperty); + restoreProperty("org.gradle.test.worker", originalWorkerProperty); + } + + private static void restoreProperty(String name, @Nullable String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } + + @Test + public void testIsEnabledFalseWhenPropertyAbsent() { + System.clearProperty(ADMIN_URL_PROPERTY); + assertFalse(SharedCoreDatabaseProvisioner.isEnabled("mysql")); + } + + @Test + public void testIsEnabledFalseWhenPropertyEmpty() { + System.setProperty(ADMIN_URL_PROPERTY, ""); + assertFalse(SharedCoreDatabaseProvisioner.isEnabled("mysql")); + } + + @Test + public void testIsEnabledTrueWhenPropertyPresent() { + System.setProperty(ADMIN_URL_PROPERTY, "jdbc:mysql://127.0.0.1:3306"); + assertTrue(SharedCoreDatabaseProvisioner.isEnabled("mysql")); + } + + @Test + public void testNextDatabaseNameIsValidAndUnique() { + System.setProperty("org.gradle.test.worker", "1"); + String first = SharedCoreDatabaseProvisioner.nextDatabaseName(); + String second = SharedCoreDatabaseProvisioner.nextDatabaseName(); + + assertTrue(first.matches("^[a-zA-Z0-9_$]+$"), "name must match the allowed character set"); + assertTrue(first.length() <= 63, "name must not exceed the 63-character cap"); + assertTrue(first.matches("^gravitino_core_[0-9a-f]+_1_\\d+$"), first); + assertNotEquals(first, second, "consecutive calls must never collide"); + } + + @Test + public void testNextDatabaseNameSanitizesWorkerId() { + // org.gradle.test.worker values are numeric in practice, but the sanitizer must still handle + // a non-alphanumeric id defensively (e.g. a manually-set property) without producing an + // invalid database name. + System.setProperty("org.gradle.test.worker", "gradle-worker-1"); + String name = SharedCoreDatabaseProvisioner.nextDatabaseName(); + assertTrue(name.matches("^[a-zA-Z0-9_$]+$")); + } + + @Test + public void testBuildJdbcUrlForDatabaseMySqlNoPath() { + String url = + SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase( + "jdbc:mysql://127.0.0.1:3306", "gravitino_core_1_1"); + assertEquals("jdbc:mysql://127.0.0.1:3306/gravitino_core_1_1", url); + } + + @Test + public void testBuildJdbcUrlForDatabasePostgresTrailingSlash() { + String url = + SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase( + "jdbc:postgresql://127.0.0.1:5432/", "gravitino_core_1_1"); + assertEquals("jdbc:postgresql://127.0.0.1:5432/gravitino_core_1_1", url); + } + + @Test + public void testBuildJdbcUrlForDatabasePreservesQueryString() { + String url = + SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase( + "jdbc:postgresql://127.0.0.1:5432/?sslmode=disable", "gravitino_core_1_1"); + assertEquals("jdbc:postgresql://127.0.0.1:5432/gravitino_core_1_1?sslmode=disable", url); + } + + @Test + public void testBuildJdbcUrlForDatabaseIpv6Host() { + String url = + SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase( + "jdbc:mysql://[::1]:3306", "gravitino_core_1_1"); + assertEquals("jdbc:mysql://[::1]:3306/gravitino_core_1_1", url); + } + + @Test + public void testBuildJdbcUrlForDatabaseReplacesExistingDatabaseName() { + String url = + SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase( + "jdbc:mysql://127.0.0.1:3306/some_other_db", "gravitino_core_1_1"); + assertEquals("jdbc:mysql://127.0.0.1:3306/gravitino_core_1_1", url); + } + + @Test + public void testBuildJdbcUrlForDatabaseHandlesSchemelessUrl() { + // No "://" at all - schemeEnd < 0, so the whole string is treated as the host part. + String url = SharedCoreDatabaseProvisioner.buildJdbcUrlForDatabase("127.0.0.1:3306", "db1"); + assertEquals("127.0.0.1:3306/db1", url); + } + + @Test + public void testNextDatabaseNameThrowsWhenWorkerIdMakesTheNameTooLong() { + // MAX_SHARED_DB_NAME_LENGTH is 63 and there is no class-name segment to truncate here (unlike + // BackendTestExtension's per-class variant), so an absurdly long worker id is the only way to + // exceed the cap - confirm it fails loudly instead of silently truncating into a name another + // worker could collide with. + System.setProperty("org.gradle.test.worker", "w".repeat(60)); + assertThrows(IllegalStateException.class, SharedCoreDatabaseProvisioner::nextDatabaseName); + } + + @Test + public void testAdminPropertyPrefixMatchesTheContractCoreBuildGradleKtsPublishesTo() { + // core/build.gradle.kts cannot import this Java test-source class at Gradle configuration + // time, so it re-declares this same prefix as a Kotlin string literal instead. Pin the + // literal here so a rename on either side that breaks the contract fails loudly (as a test + // failure) instead of silently disabling shared-container mode (isEnabled() just returns + // false when the properties never arrive, with no other signal). + assertEquals("gravitino.test.db.", SharedCoreDatabaseProvisioner.ADMIN_PROPERTY_PREFIX); + assertEquals( + "gravitino.test.db.pool.maxConnections", + SharedCoreDatabaseProvisioner.POOL_MAX_CONNECTIONS_PROPERTY); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java index 9ddbfe51c4b..abd5802a537 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaService.java @@ -35,7 +35,6 @@ import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestTemplate; public class TestJobMetaService extends TestJDBCBackend { @@ -492,51 +491,4 @@ public void testUpdateJobWithMismatchedIdThrowsIllegalArgumentException() throws .withFinishedAt(oldJob.finishedAt()) .build())); } - - @Test - public void testUpdateJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .updateJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"), e -> e)); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .updateJob( - NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX), e -> e)); - } - - @Test - public void testGetJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .getJobByIdentifier(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .getJobByIdentifier( - NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); - } - - @Test - public void testDeleteJobWithMalformedIdentifierThrowsNoSuchEntityException() { - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); - - Assertions.assertThrows( - NoSuchEntityException.class, - () -> - JobMetaService.getInstance() - .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); - } } diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java new file mode 100644 index 00000000000..2d7f6ea6176 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestJobMetaServiceValidation.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.service; + +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.job.JobHandle; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** Tests job identifier validation that does not require a JDBC backend. */ +public class TestJobMetaServiceValidation { + + private static final String METALAKE_NAME = "metalake_test_job_meta_service"; + + @Test + void testUpdateJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .updateJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"), e -> e)); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .updateJob( + NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX), e -> e)); + } + + @Test + void testGetJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .getJobByIdentifier(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .getJobByIdentifier( + NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); + } + + @Test + void testDeleteJobWithMalformedIdentifierThrowsNoSuchEntityException() { + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, "invalid"))); + + Assertions.assertThrows( + NoSuchEntityException.class, + () -> + JobMetaService.getInstance() + .deleteJob(NameIdentifierUtil.ofJob(METALAKE_NAME, JobHandle.JOB_ID_PREFIX))); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelVersionMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelVersionMetaService.java index a83f646909d..903c751128e 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelVersionMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestModelVersionMetaService.java @@ -988,7 +988,7 @@ public void testModelVersionWithMultipleUris() throws IOException { @TestTemplate public void testDeleteModelVersionsInDeletion() throws IOException, SQLException { for (String param : new String[] {"model", "schema", "catalog", "metalake"}) { - init(); + resetDatabase(); performDeletionTestLogic(param); } } diff --git a/core/src/test/java/org/apache/gravitino/testing/CoreDatabaseShardTelemetryExtension.java b/core/src/test/java/org/apache/gravitino/testing/CoreDatabaseShardTelemetryExtension.java new file mode 100644 index 00000000000..31a7bd9b954 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/testing/CoreDatabaseShardTelemetryExtension.java @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardOpenOption; +import java.util.Locale; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.LongAdder; +import java.util.function.LongSupplier; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.TestWatcher; + +/** Records class-level Core database-test evidence for each Gradle test worker. */ +public final class CoreDatabaseShardTelemetryExtension + implements BeforeAllCallback, AfterAllCallback, TestWatcher { + + static final String OUTPUT_DIRECTORY_PROPERTY = + "gravitino.core.database.shard.telemetry.directory"; + static final String BACKEND_PROPERTY = "gravitino.core.test.backend"; + + private static final Object FILE_WRITE_LOCK = new Object(); + + private final ConcurrentMap, ClassStatistics> statistics = new ConcurrentHashMap<>(); + private final LongSupplier nanoTime; + + /** Creates the extension using the system monotonic clock. */ + public CoreDatabaseShardTelemetryExtension() { + this(System::nanoTime); + } + + CoreDatabaseShardTelemetryExtension(LongSupplier nanoTime) { + this.nanoTime = nanoTime; + } + + @Override + public void beforeAll(ExtensionContext context) { + if (enabled()) { + statistics.put( + context.getRequiredTestClass(), + new ClassStatistics(context.getRequiredTestClass().getName(), nanoTime.getAsLong())); + } + } + + @Override + public void afterAll(ExtensionContext context) throws IOException { + if (!enabled()) { + return; + } + + Class testClass = context.getRequiredTestClass(); + ClassStatistics classStatistics = + statistics.computeIfAbsent( + testClass, ignored -> new ClassStatistics(testClass.getName(), nanoTime.getAsLong())); + statistics.remove(testClass); + + String backend = System.getProperty(BACKEND_PROPERTY, "unknown"); + String worker = System.getProperty("org.gradle.test.worker", "unknown"); + String record = classStatistics.toRecord(backend, worker, nanoTime.getAsLong()); + writeRecord(Paths.get(System.getProperty(OUTPUT_DIRECTORY_PROPERTY)), worker, record); + } + + @Override + public void testSuccessful(ExtensionContext context) { + statistics(context).recordPassed(); + } + + @Override + public void testAborted(ExtensionContext context, Throwable cause) { + statistics(context).recordSkipped(); + } + + @Override + public void testDisabled(ExtensionContext context, Optional reason) { + statistics(context).recordSkipped(); + } + + @Override + public void testFailed(ExtensionContext context, Throwable cause) { + statistics(context).recordFailed(); + } + + private boolean enabled() { + String outputDirectory = System.getProperty(OUTPUT_DIRECTORY_PROPERTY); + return outputDirectory != null && !outputDirectory.trim().isEmpty(); + } + + private ClassStatistics statistics(ExtensionContext context) { + Class testClass = context.getRequiredTestClass(); + return statistics.computeIfAbsent( + testClass, ignored -> new ClassStatistics(testClass.getName(), nanoTime.getAsLong())); + } + + static void writeRecord(Path outputDirectory, String worker, String record) throws IOException { + Files.createDirectories(outputDirectory); + String safeWorker = worker.replaceAll("[^A-Za-z0-9._-]", "_"); + Path workerRecord = outputDirectory.resolve("worker-" + safeWorker + ".log"); + synchronized (FILE_WRITE_LOCK) { + Files.write( + workerRecord, + (record + System.lineSeparator()).getBytes(StandardCharsets.UTF_8), + StandardOpenOption.CREATE, + StandardOpenOption.APPEND); + } + } + + static final class ClassStatistics { + private final String testClass; + private final long startNanos; + private final LongAdder passed = new LongAdder(); + private final LongAdder failed = new LongAdder(); + private final LongAdder skipped = new LongAdder(); + + ClassStatistics(String testClass, long startNanos) { + this.testClass = testClass; + this.startNanos = startNanos; + } + + void recordPassed() { + passed.increment(); + } + + void recordFailed() { + failed.increment(); + } + + void recordSkipped() { + skipped.increment(); + } + + String toRecord(String backend, String worker, long endNanos) { + long durationNanos = Math.max(0L, endNanos - startNanos); + long passedCount = passed.sum(); + long failedCount = failed.sum(); + long skippedCount = skipped.sum(); + return String.format( + Locale.ROOT, + "[CORE-DB-SHARD] backend=%s worker=%s class=%s " + + "tests=%d passed=%d failed=%d skipped=%d durationMs=%d", + backend, + worker, + testClass, + passedCount + failedCount + skippedCount, + passedCount, + failedCount, + skippedCount, + TimeUnit.NANOSECONDS.toMillis(durationNanos)); + } + } +} diff --git a/core/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseShardTelemetryExtension.java b/core/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseShardTelemetryExtension.java new file mode 100644 index 00000000000..692cee22998 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/testing/TestCoreDatabaseShardTelemetryExtension.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.testing; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicLong; +import javax.annotation.Nullable; +import org.apache.gravitino.testing.CoreDatabaseShardTelemetryExtension.ClassStatistics; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.io.TempDir; + +class TestCoreDatabaseShardTelemetryExtension { + + @Test + void formatsLeafOutcomesAndClassDuration() { + ClassStatistics statistics = new ClassStatistics("example.TestStorage", 1_000_000L); + statistics.recordPassed(); + statistics.recordPassed(); + statistics.recordFailed(); + statistics.recordSkipped(); + statistics.recordSkipped(); + + assertEquals( + "[CORE-DB-SHARD] backend=postgresql worker=2 class=example.TestStorage " + + "tests=5 passed=2 failed=1 skipped=2 durationMs=8", + statistics.toRecord("postgresql", "2", 9_000_000L)); + } + + @Test + void recordsExtensionCallbacksInWorkerFile(@TempDir Path temporaryDirectory) throws Exception { + String originalOutputDirectory = + System.getProperty(CoreDatabaseShardTelemetryExtension.OUTPUT_DIRECTORY_PROPERTY); + String originalBackend = + System.getProperty(CoreDatabaseShardTelemetryExtension.BACKEND_PROPERTY); + String originalWorker = System.getProperty("org.gradle.test.worker"); + try { + System.setProperty( + CoreDatabaseShardTelemetryExtension.OUTPUT_DIRECTORY_PROPERTY, + temporaryDirectory.toString()); + System.setProperty(CoreDatabaseShardTelemetryExtension.BACKEND_PROPERTY, "h2"); + System.setProperty("org.gradle.test.worker", "worker/7"); + + AtomicLong nanoTime = new AtomicLong(1_000_000L); + CoreDatabaseShardTelemetryExtension extension = + new CoreDatabaseShardTelemetryExtension(nanoTime::get); + ExtensionContext context = mock(ExtensionContext.class); + doReturn(SampleDatabaseTest.class).when(context).getRequiredTestClass(); + + extension.beforeAll(context); + extension.testSuccessful(context); + extension.testFailed(context, new AssertionError("expected")); + extension.testAborted(context, new IllegalStateException("expected")); + extension.testDisabled(context, Optional.of("expected")); + nanoTime.set(9_000_000L); + extension.afterAll(context); + + assertEquals( + "[CORE-DB-SHARD] backend=h2 worker=worker/7 class=" + + SampleDatabaseTest.class.getName() + + " tests=4 passed=1 failed=1 skipped=2 durationMs=8" + + System.lineSeparator(), + Files.readString(temporaryDirectory.resolve("worker-worker_7.log"))); + } finally { + restoreProperty( + CoreDatabaseShardTelemetryExtension.OUTPUT_DIRECTORY_PROPERTY, originalOutputDirectory); + restoreProperty(CoreDatabaseShardTelemetryExtension.BACKEND_PROPERTY, originalBackend); + restoreProperty("org.gradle.test.worker", originalWorker); + } + } + + private static void restoreProperty(String name, @Nullable String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } + + private static class SampleDatabaseTest {} +} diff --git a/core/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension b/core/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension new file mode 100644 index 00000000000..5f8c7a37be5 --- /dev/null +++ b/core/src/test/resources/META-INF/services/org.junit.jupiter.api.extension.Extension @@ -0,0 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +org.apache.gravitino.testing.CoreDatabaseShardTelemetryExtension diff --git a/docs/how-to-test.md b/docs/how-to-test.md index 373a510e06d..52ab7db3742 100644 --- a/docs/how-to-test.md +++ b/docs/how-to-test.md @@ -35,6 +35,38 @@ To run the unit tests, run the following command: This command runs all the unit tests and skips the integration tests. +### Core tests + +`core` tests run in three tasks, one CI job each: + +| Command | Runs | Needs Docker | +| --- | --- | --- | +| `./gradlew :core:test` | unit tests and classes tagged for H2, against H2 | no | +| `./gradlew :core:coreMySQLTest -PskipDockerTests=false` | classes tagged for MySQL | yes | +| `./gradlew :core:corePostgreSQLTest -PskipDockerTests=false` | classes tagged for PostgreSQL | yes | + +`./gradlew build` runs `:core:test`, and also the MySQL and PostgreSQL tasks when you pass `-PskipDockerTests=false`; skip those with `-x :core:coreMySQLTest -x :core:corePostgreSQLTest`. From the IDE, run a class tagged only for MySQL or PostgreSQL through its task, for example `./gradlew :core:coreMySQLTest --tests ...`. + +A class joins a database backend through `@CoreBackend.H2`, `@CoreBackend.MySQL`, `@CoreBackend.PostgreSQL` or `@CoreBackend.All`. Subclasses of `TestJDBCBackend` and `AbstractEntityStorageTest` inherit `@CoreBackend.All`. A class with none of these is a unit test and runs in `:core:test`. A class tagged `gravitino-docker-test` with none of these runs nowhere, and `TestCoreLaneMembership` fails until you add one. + +To run the MySQL or PostgreSQL tests against a database you already have (for example a service container on a Kubernetes CI runner), set its admin connection in the environment. The task then starts no container and creates a new database on that server for each test fixture: + +```shell +export GRAVITINO_TEST_MYSQL_URL='jdbc:mysql://mysql:3306/?useSSL=false&allowPublicKeyRetrieval=true' +export GRAVITINO_TEST_MYSQL_USER=root GRAVITINO_TEST_MYSQL_PASSWORD=secret +./gradlew :core:coreMySQLTest +``` + +`GRAVITINO_TEST_POSTGRESQL_URL`, `_USER` and `_PASSWORD` work the same way. The user needs permission to create databases. Without these variables the tasks start Docker containers as before. + +To check where a class runs, run it with `--tests`: + +```shell +./gradlew :core:test --tests 'org.apache.gravitino.storage.relational.service.TestYourService' +``` + +If the class does not run in that task, Gradle fails with "No tests found for given includes". In CI, `build (17)` skips `:core:test`; each core task reports under its own `core-test (17, )` check. + ## Run the Integration Tests Gravitino has two modes to run the integration tests, the default `embedded` mode and `deploy` mode. diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java index 88e4cae9392..0ad9c4dee0e 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java @@ -386,10 +386,10 @@ public void startDorisContainer(DorisImageName imageName) { } public void startMySQLContainer(TestDatabaseName testDatabaseName) { - ITUtils.cleanDisk(); if (mySQLContainer == null) { synchronized (ContainerSuite.class) { if (mySQLContainer == null) { + ITUtils.cleanDisk(); initIfNecessary(); // Start MySQL container MySQLContainer.Builder mysqlBuilder = @@ -443,10 +443,10 @@ public void startMySQLVersion5Container(TestDatabaseName testDatabaseName) { } public void startPostgreSQLContainer(TestDatabaseName testDatabaseName, PGImageName pgImageName) { - ITUtils.cleanDisk(); if (!pgContainerMap.containsKey(pgImageName)) { synchronized (ContainerSuite.class) { if (!pgContainerMap.containsKey(pgImageName)) { + ITUtils.cleanDisk(); initIfNecessary(); // Start PostgreSQL container PostgreSQLContainer.Builder pgBuilder = @@ -972,7 +972,8 @@ private static long[] cidrToRange(String cidr) throws Exception { @Override public void close() throws IOException { try { - closer.close(); + closeResources(closer); + } finally { mySQLContainer = null; mySQLVersion5Container = null; hiveContainer = null; @@ -986,8 +987,16 @@ public void close() throws IOException { kerberosHiveContainer = null; sqlBaseHiveContainer = null; pgContainerMap.clear(); + } + } + + static void closeResources(CloseableGroup resources) throws IOException { + try { + resources.close(); + } catch (IOException e) { + throw e; } catch (Exception e) { - LOG.error("Failed to close ContainerEnvironment", e); + throw new IOException("Failed to close ContainerEnvironment", e); } } diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/DatabaseProvisioning.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/DatabaseProvisioning.java new file mode 100644 index 00000000000..6d819e8d4ac --- /dev/null +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/DatabaseProvisioning.java @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.integration.test.container; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Shared, database-agnostic helper for provisioning a test database on an already-running MySQL or + * PostgreSQL container. + * + *

This class holds the validated "create database if it does not already exist" logic that used + * to live separately inside {@link MySQLContainer#createDatabase( + * org.apache.gravitino.integration.test.util.TestDatabaseName)} and {@link + * PostgreSQLContainer#createDatabase(org.apache.gravitino.integration.test.util.TestDatabaseName)}. + * Both container classes now delegate to {@link #createDatabaseIfAbsent(String, String, String, + * String)}, and any other caller (for example a shared-container test extension) that only has a + * JDBC admin URL, credentials, and a database name can use it directly without going through a + * {@code TestDatabaseName} enum value or a container instance. + */ +public class DatabaseProvisioning { + + private static final Logger LOG = LoggerFactory.getLogger(DatabaseProvisioning.class); + + private static final int MAX_DATABASE_NAME_LENGTH = 63; + + private static final String VALID_DATABASE_NAME_REGEX = "^[a-zA-Z0-9_$]+$"; + + private static final String MYSQL_JDBC_URL_PREFIX = "jdbc:mysql:"; + + private static final String POSTGRESQL_JDBC_URL_PREFIX = "jdbc:postgresql:"; + + /** PostgreSQL's SQLSTATE for "a database with this name already exists". */ + private static final String POSTGRESQL_DUPLICATE_DATABASE_SQLSTATE = "42P04"; + + // Private constructor to prevent instantiation of this static helper class. + private DatabaseProvisioning() {} + + /** + * Creates a database on the target MySQL or PostgreSQL server if it does not already exist. + * + *

The dialect (MySQL vs. PostgreSQL) is inferred from the scheme of {@code adminJdbcUrl} + * ({@code jdbc:mysql:...} or {@code jdbc:postgresql:...}). {@code adminJdbcUrl} must be a JDBC + * URL that can be connected to without naming {@code dbName} itself, e.g. the server root URL or + * another already-existing database on the same server. + * + *

For MySQL this issues {@code CREATE DATABASE IF NOT EXISTS }. PostgreSQL has no + * {@code IF NOT EXISTS} clause for {@code CREATE DATABASE}, so this first checks {@code + * pg_database} for an existing row and only issues {@code CREATE DATABASE ""} when none + * is found. + * + * @param adminJdbcUrl a JDBC URL that can be connected to without naming {@code dbName} + * @param user the database user used to connect and create the database + * @param password the password for {@code user} + * @param dbName the name of the database to create; validated against {@link + * #isValidDatabaseName(String)} + * @throws IllegalArgumentException if {@code dbName} is invalid, or {@code adminJdbcUrl} is + * neither a MySQL nor a PostgreSQL JDBC URL + * @throws RuntimeException if the database cannot be created + *

Note for existing {@link MySQLContainer#createDatabase}/{@link + * PostgreSQLContainer#createDatabase} callers migrated onto this helper: the accepted name + * length cap is 63 characters (PostgreSQL's identifier limit), tighter than the 64 {@code + * MySQLContainer} previously allowed on its own; every {@code TestDatabaseName} value in this + * repo is well under that either way. An invalid name now surfaces as a bare {@link + * IllegalArgumentException} rather than one wrapped in a {@code RuntimeException}. + */ + public static void createDatabaseIfAbsent( + String adminJdbcUrl, String user, String password, String dbName) { + if (!isValidDatabaseName(dbName)) { + throw new IllegalArgumentException("Invalid database name: " + dbName); + } + + if (adminJdbcUrl.startsWith(MYSQL_JDBC_URL_PREFIX)) { + createMySQLDatabaseIfAbsent(adminJdbcUrl, user, password, dbName); + } else if (adminJdbcUrl.startsWith(POSTGRESQL_JDBC_URL_PREFIX)) { + createPostgreSQLDatabaseIfAbsent(adminJdbcUrl, user, password, dbName); + } else { + throw new IllegalArgumentException( + "Unsupported JDBC URL for database provisioning: " + adminJdbcUrl); + } + } + + /** + * Validates that {@code databaseName} only contains safe characters and does not exceed the + * shared length cap, so it can be interpolated into a {@code CREATE DATABASE} statement. + * + * @param databaseName the candidate database name + * @return {@code true} if the name is non-null, non-empty, at most {@value + * #MAX_DATABASE_NAME_LENGTH} characters, and matches {@code ^[a-zA-Z0-9_$]+$} + */ + public static boolean isValidDatabaseName(String databaseName) { + if (databaseName == null || databaseName.isEmpty()) { + return false; + } + + if (databaseName.length() > MAX_DATABASE_NAME_LENGTH) { + return false; + } + + return databaseName.matches(VALID_DATABASE_NAME_REGEX); + } + + private static void createMySQLDatabaseIfAbsent( + String adminJdbcUrl, String user, String password, String dbName) { + try (Connection connection = DriverManager.getConnection(adminJdbcUrl, user, password); + Statement statement = connection.createStatement()) { + executeCreateMySQLDatabase(statement, dbName); + } catch (SQLException e) { + throw new RuntimeException("Failed to create MySQL database " + dbName, e); + } + } + + /** + * Issues the MySQL {@code CREATE DATABASE IF NOT EXISTS} statement against an already-open {@link + * Statement}. Package-private (rather than folded into {@link #createMySQLDatabaseIfAbsent}) so + * it can be unit-tested against a mocked {@link Statement} without a real database connection. + */ + static void executeCreateMySQLDatabase(Statement statement, String dbName) throws SQLException { + // FIXME: dbName is validated by isValidDatabaseName(), but interpolating it into SQL is + // still not ideal. + statement.execute(String.format("CREATE DATABASE IF NOT EXISTS `%s`", dbName)); + LOG.info("MySQL database {} has been created (or already existed)", dbName); + } + + private static void createPostgreSQLDatabaseIfAbsent( + String adminJdbcUrl, String user, String password, String dbName) { + try (Connection connection = DriverManager.getConnection(adminJdbcUrl, user, password); + Statement statement = connection.createStatement()) { + executeCreatePostgreSQLDatabase(statement, dbName); + } catch (SQLException e) { + throw new RuntimeException("Failed to create PostgreSQL database " + dbName, e); + } + } + + /** + * Issues the PostgreSQL exists-check-then-{@code CREATE DATABASE} sequence against an + * already-open {@link Statement}, tolerating SQLSTATE {@value + * #POSTGRESQL_DUPLICATE_DATABASE_SQLSTATE} (duplicate database) as success. Package-private + * (rather than folded into {@link #createPostgreSQLDatabaseIfAbsent}) so it can be unit-tested + * against a mocked {@link Statement}/{@link ResultSet} without a real database connection. + */ + static void executeCreatePostgreSQLDatabase(Statement statement, String dbName) + throws SQLException { + if (postgreSQLDatabaseExists(statement, dbName)) { + LOG.info("PostgreSQL database {} already exists, skipping creation", dbName); + return; + } + + // PostgreSQL's CREATE DATABASE has no IF NOT EXISTS clause, so existence is checked above. + // That check-then-create is not race-safe on its own (another caller could create dbName + // between the check and this statement), so a duplicate-database error here is still + // treated as success rather than propagated. + try { + statement.execute(String.format("CREATE DATABASE \"%s\"", dbName)); + LOG.info("PostgreSQL database {} has been created", dbName); + } catch (SQLException e) { + if (POSTGRESQL_DUPLICATE_DATABASE_SQLSTATE.equals(e.getSQLState())) { + LOG.info( + "PostgreSQL database {} was created concurrently by another caller, skipping", dbName); + return; + } + throw e; + } + } + + static boolean postgreSQLDatabaseExists(Statement statement, String dbName) throws SQLException { + String query = String.format("SELECT 1 FROM pg_database WHERE datname = '%s'", dbName); + try (ResultSet resultSet = statement.executeQuery(query)) { + return resultSet.next(); + } + } +} diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/MySQLContainer.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/MySQLContainer.java index 3cfb6ecc56b..c17a5b689ce 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/MySQLContainer.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/MySQLContainer.java @@ -22,10 +22,8 @@ import static org.testcontainers.shaded.org.awaitility.Awaitility.await; import com.google.common.collect.ImmutableSet; -import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; -import java.sql.Statement; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -128,35 +126,8 @@ public void createDatabase(TestDatabaseName testDatabaseName) { } // change password for root user, Gravitino API must set password in catalog properties - try (Connection connection = - DriverManager.getConnection(mySQLJdbcUrl, USER_NAME, getPassword()); - Statement statement = connection.createStatement()) { - - // validate database name to ensure it only contains safe characters - String databaseName = testDatabaseName.toString(); - if (!isValidDatabaseName(databaseName)) { - throw new IllegalArgumentException("Invalid database name: " + databaseName); - } - - String query = String.format("CREATE DATABASE IF NOT EXISTS `%s`;", databaseName); - // FIXME: String, which is used in SQL, can be unsafe - statement.execute(query); - LOG.info(String.format("MySQL container database %s has been created", testDatabaseName)); - } catch (Exception e) { - throw new RuntimeException("Failed to create database", e); - } - } - - private boolean isValidDatabaseName(String databaseName) { - if (databaseName == null || databaseName.isEmpty()) { - return false; - } - - if (databaseName.length() > 64) { - return false; - } - - return databaseName.matches("^[a-zA-Z0-9_$]+$"); + DatabaseProvisioning.createDatabaseIfAbsent( + mySQLJdbcUrl, USER_NAME, getPassword(), testDatabaseName.toString()); } public String getUsername() { diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/PostgreSQLContainer.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/PostgreSQLContainer.java index 6b2b147b1c8..008e30fb7c9 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/PostgreSQLContainer.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/PostgreSQLContainer.java @@ -22,10 +22,8 @@ import static org.testcontainers.shaded.org.awaitility.Awaitility.await; import com.google.common.collect.ImmutableSet; -import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; -import java.sql.Statement; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -112,19 +110,8 @@ protected boolean checkContainerStatus(int retryLimit) { } public void createDatabase(TestDatabaseName testDatabaseName) { - try (Connection connection = - DriverManager.getConnection(getJdbcUrl(), getUsername(), getPassword()); - Statement statement = connection.createStatement()) { - - String query = format("CREATE DATABASE \"%s\"", testDatabaseName); - statement.execute(query); - LOG.info(format("PostgreSQL container database %s has been created", testDatabaseName)); - } catch (SQLException e) { - if (e.getMessage() - .equals(String.format("ERROR: database \"%s\" already exists", testDatabaseName))) { - LOG.info("PostgreSQL Database {} already exists, skipping", testDatabaseName); - } else throw new RuntimeException(e); - } + DatabaseProvisioning.createDatabaseIfAbsent( + getJdbcUrl(), getUsername(), getPassword(), testDatabaseName.toString()); } public String getUsername() { diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java new file mode 100644 index 00000000000..4c7a21b934e --- /dev/null +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestContainerSuite.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.integration.test.container; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import org.apache.gravitino.integration.test.util.CloseableGroup; +import org.junit.jupiter.api.Test; + +class TestContainerSuite { + + @Test + void testCloseResourcesPropagatesFailureAfterClosingEveryResource() { + CloseableGroup resources = CloseableGroup.create(); + int[] closeCount = new int[1]; + resources.register(() -> closeCount[0]++); + resources.register( + () -> { + closeCount[0]++; + throw new IOException("expected"); + }); + + IOException failure = + assertThrows(IOException.class, () -> ContainerSuite.closeResources(resources)); + + assertEquals("expected", failure.getMessage()); + assertEquals(2, closeCount[0]); + } +} diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestDatabaseProvisioning.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestDatabaseProvisioning.java new file mode 100644 index 00000000000..f630cc57a77 --- /dev/null +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/TestDatabaseProvisioning.java @@ -0,0 +1,182 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.integration.test.container; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link DatabaseProvisioning}'s pure validation/dispatch logic, plus the PostgreSQL + * exists-check/duplicate-tolerant create sequence and the MySQL statement it issues, both exercised + * here against a mocked {@link Statement} rather than a live server. The connect-then-delegate + * paths ({@code createMySQLDatabaseIfAbsent}/{@code createPostgreSQLDatabaseIfAbsent}) still need a + * live server and are exercised indirectly by the docker-tagged integration tests instead. + */ +public class TestDatabaseProvisioning { + + @Test + public void testIsValidDatabaseNameAcceptsAllowedCharacters() { + assertTrue(DatabaseProvisioning.isValidDatabaseName("gravitino_core_w1")); + assertTrue(DatabaseProvisioning.isValidDatabaseName("a")); + assertTrue(DatabaseProvisioning.isValidDatabaseName("A1_$")); + } + + @Test + public void testIsValidDatabaseNameRejectsNullOrEmpty() { + assertFalse(DatabaseProvisioning.isValidDatabaseName(null)); + assertFalse(DatabaseProvisioning.isValidDatabaseName("")); + } + + @Test + public void testIsValidDatabaseNameRejectsDisallowedCharacters() { + assertFalse(DatabaseProvisioning.isValidDatabaseName("bad-name")); + assertFalse(DatabaseProvisioning.isValidDatabaseName("bad name")); + assertFalse(DatabaseProvisioning.isValidDatabaseName("bad;name")); + assertFalse(DatabaseProvisioning.isValidDatabaseName("bad'name")); + } + + @Test + public void testIsValidDatabaseNameRejectsNamesOverSixtyThreeCharacters() { + String sixtyThree = "a".repeat(63); + String sixtyFour = "a".repeat(64); + assertTrue(DatabaseProvisioning.isValidDatabaseName(sixtyThree)); + assertFalse(DatabaseProvisioning.isValidDatabaseName(sixtyFour)); + } + + @Test + public void testCreateDatabaseIfAbsentRejectsInvalidName() { + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> + DatabaseProvisioning.createDatabaseIfAbsent( + "jdbc:mysql://127.0.0.1:3306", "root", "root", "bad-name")); + assertTrue(e.getMessage().contains("Invalid database name")); + } + + @Test + public void testCreateDatabaseIfAbsentRejectsUnsupportedJdbcUrl() { + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> + DatabaseProvisioning.createDatabaseIfAbsent( + "jdbc:h2:mem:test", "root", "root", "valid_name")); + assertTrue(e.getMessage().contains("Unsupported JDBC URL")); + } + + @Test + public void testExecuteCreateMySQLDatabaseIssuesCreateIfNotExists() throws SQLException { + Statement statement = mock(Statement.class); + DatabaseProvisioning.executeCreateMySQLDatabase(statement, "gravitino_core_1"); + verify(statement).execute("CREATE DATABASE IF NOT EXISTS `gravitino_core_1`"); + } + + @Test + public void testPostgreSQLDatabaseExistsTrueWhenQueryReturnsARow() throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(contains("gravitino_core_1"))).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); + + assertTrue(DatabaseProvisioning.postgreSQLDatabaseExists(statement, "gravitino_core_1")); + } + + @Test + public void testPostgreSQLDatabaseExistsFalseWhenQueryReturnsNoRow() throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + assertFalse(DatabaseProvisioning.postgreSQLDatabaseExists(statement, "gravitino_core_1")); + } + + @Test + public void testExecuteCreatePostgreSQLDatabaseSkipsCreateWhenAlreadyExists() + throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); // exists-check finds a row + + DatabaseProvisioning.executeCreatePostgreSQLDatabase(statement, "gravitino_core_1"); + + verify(statement, never()).execute(anyString()); + } + + @Test + public void testExecuteCreatePostgreSQLDatabaseIssuesCreateWhenAbsent() throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); // exists-check finds no row + + DatabaseProvisioning.executeCreatePostgreSQLDatabase(statement, "gravitino_core_1"); + + verify(statement).execute("CREATE DATABASE \"gravitino_core_1\""); + } + + @Test + public void testExecuteCreatePostgreSQLDatabaseTreatsDuplicateSqlStateAsSuccess() + throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + // 42P04 = PostgreSQL's duplicate_database SQLSTATE: another caller won the race between this + // method's own exists-check and its CREATE DATABASE statement. + when(statement.execute(anyString())) + .thenThrow(new SQLException("database already exists", "42P04")); + + // Must not throw. + DatabaseProvisioning.executeCreatePostgreSQLDatabase(statement, "gravitino_core_1"); + } + + @Test + public void testExecuteCreatePostgreSQLDatabaseRethrowsNonDuplicateSqlState() + throws SQLException { + Statement statement = mock(Statement.class); + ResultSet resultSet = mock(ResultSet.class); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + SQLException permissionDenied = new SQLException("permission denied", "42501"); + when(statement.execute(anyString())).thenThrow(permissionDenied); + + SQLException thrown = + assertThrows( + SQLException.class, + () -> + DatabaseProvisioning.executeCreatePostgreSQLDatabase( + statement, "gravitino_core_1")); + assertTrue(thrown == permissionDenied); + } +} diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java index a37a9647009..51becc11527 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/CloseContainerExtension.java @@ -32,14 +32,11 @@ public class CloseContainerExtension implements BeforeAllCallback { @Override public void beforeAll(ExtensionContext extensionContext) { - // Ensure that the container suite is initialized before closing it - if (ContainerSuite.initialized()) { - synchronized (CloseContainerExtension.class) { - extensionContext - .getRoot() - .getStore(ExtensionContext.Namespace.GLOBAL) - .getOrComputeIfAbsent(CloseableContainer.class); - } + synchronized (CloseContainerExtension.class) { + extensionContext + .getRoot() + .getStore(ExtensionContext.Namespace.GLOBAL) + .getOrComputeIfAbsent(CloseableContainer.class); } } @@ -47,6 +44,8 @@ private static class CloseableContainer implements ExtensionContext.Store.Closea private static final Logger LOGGER = LoggerFactory.getLogger(CloseableContainer.class); private static final ContainerSuite CONTAINER_SUITE = ContainerSuite.getInstance(); + // Log instead of failing: this runs for every integration-test module at the end of the plan, + // and a slow container teardown must not fail an otherwise green run. @Override public void close() { try {