wombatu-kun commented on code in PR #19953: URL: https://github.com/apache/hudi/pull/19953#discussion_r4022111253
########## docker/trino/build_trino_server_image.sh: ########## @@ -0,0 +1,144 @@ +#!/bin/bash +# 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. + +## +## Builds a local Trino server image from the trinodb/trino commit pinned by +## <trino.sha> in the root pom, so the server and the hudi-trino connector baked +## on top of it (docker/trino/build_image.sh --base-image) come from the same +## commit. The image is local-only and never published. +## +## Usage: build_trino_server_image.sh <path-to-trino-checkout> [--image <name:tag>] [--arch <amd64|arm64>] +## +## <path-to-trino-checkout> a trinodb/trino git checkout at exactly <trino.sha> +## --image <name:tag> tag for the resulting image +## (default: hudi-trino-server:<trino.sha>) +## --arch <amd64|arm64> image architecture (default: the Docker daemon's arch) +## +## Requires JDK 25, jq and docker on PATH. +## + +set -euo pipefail + +HUDI_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" + +TRINO_REPO="" +IMAGE="" +ARCH="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --image) + IMAGE="$2" + shift 2 + ;; + --arch) + ARCH="$2" + shift 2 + ;; + -h|--help) + grep '^##' "$0" | sed 's/^## \{0,1\}//' + exit 0 + ;; + *) + if [[ -z "$TRINO_REPO" ]]; then + TRINO_REPO="$1" + shift + else + echo "ERROR: unexpected argument: $1" >&2 + exit 1 + fi + ;; + esac +done + +if [[ -z "$TRINO_REPO" || ! -d "$TRINO_REPO/.git" && ! -f "$TRINO_REPO/.git" ]]; then + echo "ERROR: first argument must be a trinodb/trino git checkout" >&2 + echo "Usage: $0 <path-to-trino-checkout> [--image <name:tag>] [--arch <amd64|arm64>]" >&2 + exit 1 +fi +TRINO_REPO="$(cd "$TRINO_REPO" && pwd)" + +# Keep each property on one line in the root pom; this sed depends on it. +TRINO_SHA=$(sed -n 's|.*<trino.sha>\(.*\)</trino.sha>.*|\1|p' "$HUDI_ROOT/pom.xml") +TRINO_VERSION=$(sed -n 's|.*<trino.version>\(.*\)</trino.version>.*|\1|p' "$HUDI_ROOT/pom.xml") +if [[ -z "$TRINO_SHA" || -z "$TRINO_VERSION" ]]; then + echo "ERROR: could not read <trino.sha>/<trino.version> from $HUDI_ROOT/pom.xml" >&2 + exit 1 +fi + +# JDK gate: trino at head enforces JDK 25. +JAVA_MAJOR=$(java -version 2>&1 | awk -F[\".] '/version/ {print $2}') +if [[ "$JAVA_MAJOR" != "25" ]]; then + echo "ERROR: JDK 25 required to build trino (found major version: ${JAVA_MAJOR:-unknown})." >&2 + echo "Hint: export JAVA_HOME=\$(/usr/libexec/java_home -v 25)" >&2 + exit 1 +fi + +for tool in jq docker; do + if ! command -v "$tool" > /dev/null 2>&1; then + echo "ERROR: $tool is required on PATH (trino's core/docker/build.sh needs jq and docker)" >&2 + exit 1 + fi +done + +ACTUAL_SHA=$(git -C "$TRINO_REPO" rev-parse HEAD) +if [[ "$ACTUAL_SHA" != "$TRINO_SHA" ]]; then + echo "ERROR: $TRINO_REPO is at $ACTUAL_SHA, but the root pom pins <trino.sha>$TRINO_SHA</trino.sha>." >&2 + echo "Hint: git -C $TRINO_REPO -c advice.detachedHead=false checkout --detach $TRINO_SHA" >&2 + exit 1 +fi + +ACTUAL_VERSION=$("$TRINO_REPO/mvnw" -q -N -f "$TRINO_REPO/pom.xml" help:evaluate -Dexpression=project.version -DforceStdout) +if [[ "$ACTUAL_VERSION" != "$TRINO_VERSION" ]]; then + echo "ERROR: trino at pinned sha $TRINO_SHA has version $ACTUAL_VERSION, but the root pom says <trino.version>$TRINO_VERSION</trino.version>." >&2 + echo "The pin properties must advance together; fix the pom or your checkout." >&2 + exit 1 +fi + +if [[ -z "$ARCH" ]]; then + ARCH=$(docker version --format '{{.Server.Arch}}') +fi +if [[ "$ARCH" != "amd64" && "$ARCH" != "arm64" ]]; then + echo "ERROR: --arch must be amd64 or arm64 (got: ${ARCH:-empty})" >&2 + exit 1 +fi + +if [[ -z "$IMAGE" ]]; then + IMAGE="hudi-trino-server:${TRINO_SHA}" +fi + +export MAVEN_OPTS="${MAVEN_OPTS:--Xmx4g}" Review Comment: Trino's `.mvn/jvm.config` sets `-Xmx8192m` with `-XX:+ExitOnOutOfMemoryError`, and Maven's launcher puts `MAVEN_OPTS` last, so this default halves the heap for the whole-repo build below. Use `-Xmx8g`, or drop the line so Trino's own `jvm.config` stands. ########## .github/workflows/hudi_trino_e2e.yml: ########## @@ -76,135 +78,83 @@ jobs: set -euo pipefail TRINO_SHA=$(sed -n 's|.*<trino.sha>\(.*\)</trino.sha>.*|\1|p' pom.xml) TRINO_VERSION=$(sed -n 's|.*<trino.version>\(.*\)</trino.version>.*|\1|p' pom.xml) - E2E_VERSION=$(sed -n 's|.*<trino.e2e.version>\(.*\)</trino.e2e.version>.*|\1|p' pom.xml) - # sed -n ...p exits 0 on no match; an empty sha would make the commits API answer - # for the default branch and silently skip the suite, so fail loudly instead. - if [ -z "$TRINO_SHA" ] || [ -z "$TRINO_VERSION" ] || [ -z "$E2E_VERSION" ]; then - echo "ERROR: could not read trino.sha/trino.version/trino.e2e.version from pom.xml" >&2 + # sed -n ...p exits 0 on no match; an empty sha would check out trinodb/trino's default + # branch and key the caches on nothing, so fail loudly instead. + if [ -z "$TRINO_SHA" ] || [ -z "$TRINO_VERSION" ]; then + echo "ERROR: could not read trino.sha/trino.version from pom.xml" >&2 exit 1 fi - echo "Connector builds at $TRINO_VERSION ($TRINO_SHA); server image is $E2E_VERSION" + echo "Server image and connector both build at $TRINO_VERSION ($TRINO_SHA)" echo "trino_sha=$TRINO_SHA" >> "$GITHUB_OUTPUT" echo "trino_version=$TRINO_VERSION" >> "$GITHUB_OUTPUT" - echo "e2e_version=$E2E_VERSION" >> "$GITHUB_OUTPUT" - - name: SPI drift gate - id: spi-drift - # The plugin is built at the pin but loaded by the released trino.e2e.version server, so - # any SPI / filesystem change between the two can make the image unbootable. Skip the run - # instead of reporting a failure that no connector change caused. - env: - GH_TOKEN: ${{ github.token }} - TRINO_SHA: ${{ steps.trino-pin.outputs.trino_sha }} - TRINO_VERSION: ${{ steps.trino-pin.outputs.trino_version }} - E2E_VERSION: ${{ steps.trino-pin.outputs.e2e_version }} - run: | - set -euo pipefail - # Per-path commit queries, NOT the compare API: compare caps its file list at 300 - # and a single Trino release cycle already exceeds that, so a capped compare would - # flag every pin more than a release old as drifted. The commits API is uncapped; - # any commit reachable from the pin that touched a boundary-crossing path after the - # released tag's commit date (excluding the tag commit itself) is drift. Existence - # is enough, so the first page settles it -- truncation cannot yield a false pass. - # Gated paths are the surfaces where server-built and pin-built classes meet: - # core/trino-spi (the server provides it to the plugin classloader), and the - # HdfsFileSystemLoader contract, where bundled trino-filesystem-manager loads the - # server image's version-matched hdfs jar set (see docker/trino/Dockerfile) whose - # HdfsClassLoader then delegates the exact packages io.trino.filesystem and - # io.trino.memory.context back to the plugin's bundled copies -- so those two libs - # cross the boundary precisely BECAUSE they are bundled. - TAG_SHA=$(gh api "repos/trinodb/trino/commits/${E2E_VERSION}" --jq .sha) - TAG_DATE=$(gh api "repos/trinodb/trino/commits/${E2E_VERSION}" --jq .commit.committer.date) - DRIFTED=false - for p in core/trino-spi lib/trino-filesystem lib/trino-filesystem-manager lib/trino-hdfs lib/trino-memory-context; do - # Assign before iterating: a failing substitution in the for-list would not trip - # set -e, and gh api prints the error body to stdout, so a transient API error - # would otherwise iterate over error JSON and silently skip the suite. - SHAS=$(gh api "repos/trinodb/trino/commits?sha=${TRINO_SHA}&path=${p}&since=${TAG_DATE}" --jq '.[].sha') - for c in $SHAS; do - if [ "$c" != "$TAG_SHA" ]; then - echo "Boundary-crossing change under ${p}: ${c}" - DRIFTED=true - fi - done - done - echo "drifted=$DRIFTED" >> "$GITHUB_OUTPUT" - if [ "$DRIFTED" = "true" ]; then - { - echo "## Trino E2E skipped: SPI drift window" - echo "" - echo "The connector is built against trinodb/trino \`${TRINO_VERSION}\` (\`${TRINO_SHA}\`), while the" - echo "e2e server image is released Trino \`${E2E_VERSION}\`. SPI / filesystem paths changed between" - echo "the two, so this run is skipped until the pin and the released version re-align." - } >> "$GITHUB_STEP_SUMMARY" - fi - name: Free disk space - if: steps.spi-drift.outputs.drifted != 'true' run: | sudo rm -rf /usr/share/dotnet sudo rm -rf /usr/local/lib/android sudo rm -rf /opt/ghc sudo rm -rf /usr/local/share/boost docker system prune --all --force --volumes - name: Pre-pull compose images (fails fast if not published) - if: steps.spi-drift.outputs.drifted != 'true' run: | # Surface a missing sparkadhoc image before the long Maven install. The # remaining stack images are pulled by docker-compose at test time; the # trino image is built locally below, never pulled. docker pull apachehudi/hudi-hadoop_3.4.0-hive_2.3.10-sparkadhoc_4.0.2:latest - name: Set up JDK 17 - if: steps.spi-drift.outputs.drifted != 'true' uses: actions/setup-java@v5 with: java-version: '17' distribution: 'temurin' architecture: x64 cache: maven - name: Build and install Hudi artifacts (JDK 17) - if: steps.spi-drift.outputs.drifted != 'true' # Full reactor: the compose containers mount the workspace and the tests # use bundles staged by the -Pintegration-tests build (e.g. # docker/hoodie/hadoop/hive_base/target/hoodie-spark-bundle.jar). run: mvn clean install -T 2 $SCALA_PROFILE -Dspark4.0 -Dflink1.20 -Pintegration-tests -DskipTests=true -Ddocker.compose.skip=true $MVN_ARGS - name: Set up JDK 25 - if: steps.spi-drift.outputs.drifted != 'true' uses: actions/setup-java@v5 with: java-version: '25' distribution: 'temurin' cache: maven - name: Purge Trino artifacts from the local m2 - if: steps.spi-drift.outputs.drifted != 'true' # Artifacts an older pin left behind carry the same SNAPSHOT coordinates as the current ones. run: rm -rf ~/.m2/repository/io/trino # The connector and the shim assembly resolve io.trino from the pinned trinodb/trino # commit: Trino publishes no SNAPSHOT artifacts, so nothing here comes from Central. - name: Restore Trino artifacts for the pinned commit id: trino-m2 - if: steps.spi-drift.outputs.drifted != 'true' uses: actions/cache@v4 with: path: ~/.m2/repository/io/trino key: trino-m2-v2-${{ hashFiles('scripts/trino/bootstrap_trino.sh') }}-${{ steps.trino-pin.outputs.trino_sha }} # No restore-keys on purpose: a partial restore from another pin collides on the # same SNAPSHOT coordinates and poisons the build. - name: Checkout trinodb/trino at the pinned commit - if: steps.spi-drift.outputs.drifted != 'true' && steps.trino-m2.outputs.cache-hit != 'true' uses: actions/checkout@v5 with: repository: trinodb/trino ref: ${{ steps.trino-pin.outputs.trino_sha }} path: trino-src - name: Build Trino artifacts from source (JDK 25) - if: steps.spi-drift.outputs.drifted != 'true' && steps.trino-m2.outputs.cache-hit != 'true' + if: steps.trino-m2.outputs.cache-hit != 'true' Review Comment: The `delete-all-caches` purge empties this key too, so this gate is always true and a hit would leave the server-image step below to cold-build the whole Trino repo. Drop this cache as well, or keep it for a follow-up? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
