wombatu-kun commented on code in PR #19642:
URL: https://github.com/apache/hudi/pull/19642#discussion_r3794490055
##########
.github/workflows/hudi_trino_ci.yml:
##########
@@ -127,20 +139,32 @@ jobs:
java-version: '25'
distribution: 'temurin'
cache: maven
- # Trino does not publish trino-spi / trino-filesystem / trino-hive
test-jars to
- # Maven Central. Check out the matching release tag and install just the
modules
- # whose test classifiers we need into the local m2.
- - name: Checkout trinodb/trino at 481
+ - name: Purge Trino artifacts from the local m2
if: needs.changes.outputs.trino == 'true'
+ # Artifacts an older pin left behind carry the same SNAPSHOT
coordinates as the current ones.
+ run: rm -rf ~/.m2/repository/io/trino
+ # Trino publishes neither SNAPSHOT artifacts nor the trino-spi /
trino-filesystem /
+ # trino-hive / trino-main test-jars, so every io.trino dependency is
built from the
+ # pinned trinodb/trino commit and cached under that commit.
+ - name: Restore Trino artifacts for the pinned commit
+ id: trino-m2
+ if: needs.changes.outputs.trino == 'true'
+ uses: actions/cache@v4
+ with:
+ path: ~/.m2/repository/io/trino
+ key: trino-m2-v2-${{ steps.trino-pin.outputs.trino_sha }}
Review Comment:
This key covers the pin but not `bootstrap_trino.sh`'s `-pl` list, which is
what actually determines the cached contents, and the bootstrap step is skipped
on a cache hit - so a PR that edits the module list without moving the pin can
never exercise it. Add `-${{ hashFiles('scripts/trino/bootstrap_trino.sh') }}`
to the key here and in the e2e and compat workflows so all three stay in step.
##########
.github/workflows/hudi_trino_compat.yml:
##########
@@ -59,18 +82,27 @@ jobs:
echo "trino_version=$VERSION" >> "$GITHUB_OUTPUT"
echo "Detected Trino version: $VERSION"
- name: Install Trino modules from master (JDK 25)
- working-directory: trino
- # hudi-trino compiles against these plus their transitive modules
(spi, cache, metastore,
- # hive-formats, memory-context). They must come from the master
checkout -- resolving from
- # Maven Central would defeat the point of the drift check.
- run: mvn $MVN_ARGS install -pl
:trino-hive,:trino-filesystem-manager,:trino-parquet,:trino-plugin-toolkit -am
-DskipTests -Dair.check.skip-all=true
+ # Same module set the pinned build uses, so a green compile here is a
promotable pin. The
+ # script purges ~/.m2/repository/io/trino itself and only warns when
master's version has
+ # rolled past the pinned trino.version.
+ env:
+ HEAD_SHA: ${{ steps.trino-head.outputs.head_sha }}
+ run: hudi/scripts/trino/bootstrap_trino.sh trino --skip-checkout --ref
"$HEAD_SHA"
Review Comment:
This job only runs `compile` on hudi-trino, so drift that breaks just the
test sources reports green, files no drift issue and still lets
`propose-pin-advance` push the pin. Add a `-Phudi-trino,hudi-trino-tests -pl
hudi-trino test` step here - the bootstrap already installs the four test-jars,
and the JDK 17 install would need `hudi-client-common` and `hudi-java-client`
the way `hudi_trino_ci.yml` does.
##########
.github/workflows/hudi_trino_e2e.yml:
##########
@@ -62,50 +64,139 @@ jobs:
# hudi-trino at HEAD, assembles the plugin dir via the in-repo shim
# (docker/trino/shim, standing in for the not-yet-released upstream
# trinodb/trino plugin/trino-hudi shim), bakes it into a local
- # apachehudi/hudi-trino_481 image, and runs ITTestTrino* against the
- # spark402 compose stack (the only pair with the trinocoordinator service).
+ # apachehudi/hudi-trino-e2e image on top of the released trino.e2e.version
+ # server, and runs ITTestTrino* against the spark402 compose stack (the
only
+ # pair with the trinocoordinator service).
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5
+ - name: Read Trino pin
+ id: trino-pin
+ run: |
+ 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)
+ echo "Connector builds at $TRINO_VERSION ($TRINO_SHA); server image
is $E2E_VERSION"
+ 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.
+ # Only two surfaces cross the plugin/server boundary and are
therefore gated:
+ # core/trino-spi (the server provides it to the plugin classloader)
and the
+ # reflective HdfsFileSystemLoader contract (bundled
trino-filesystem-manager loads
+ # the server image's version-matched hdfs jar set; see
docker/trino/Dockerfile).
+ # lib/trino-filesystem ships inside the plugin dir, so it cannot
skew the boot.
+ 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-manager lib/trino-hdfs;
do
Review Comment:
`HdfsClassLoader` routes the exact package `io.trino.filesystem` to the
plugin's own classloader, so the server image's `<plugin>/hdfs` jars bind
against the pin's copy of it - being bundled is why it crosses the boundary,
not why it doesn't. Put `lib/trino-filesystem` back in the gated list and drop
the claim that it cannot skew the boot.
##########
release/release_guide.md:
##########
@@ -290,6 +290,31 @@ Here is how to go about a bug fix release.
- Go to apache/hudi repo locally and pull this branch. Here after you can work
on this branch and push to origin when need be.
- Do not forget to set the env variables from above section.
+## hudi-trino Trino pin-back
+
+On master hudi-trino tracks `trinodb/trino` master at the commit in
`trino.sha`, whose `trino.version` is a
+`-SNAPSHOT` that resolves from nowhere but a local build. A release must
depend on a released Trino, and the pin-back
+must land on the release branch before the source release is generated (see
"Build a release candidate", the Generate
+Source Release step) -- otherwise the voted tarball ships a `-SNAPSHOT` Trino
pin that cannot be built from Central.
+
+1. Wait for the latest released Trino `NNN` to be available on Maven Central.
+2. In a `trinodb/trino` checkout, find the tagged commit: `TAG_SHA=$(git
rev-list -n1 NNN)`.
+3. If the pin is behind the tag, advance master's pin to `TAG_SHA` first by
dispatching the
+ `Hudi Trino SPI Compatibility` workflow with `trino_ref=NNN` (it then
verifies and pins exactly that tag rather
+ than master HEAD) and merging the pin PR a committer opens from the pushed
`bot/trino-pin` branch. If the pin is ahead of the tag, enumerate the
adaptations that would be lost with
+ `git log NNN..<pin> -- core/trino-spi lib/trino-filesystem
lib/trino-filesystem-manager lib/trino-hdfs`
+ and revert them forward on the release branch only, never on master.
+4. On the release branch set `trino.version=NNN`, `trino.sha=TAG_SHA` and
`trino.e2e.version=NNN` in the root
+ pom, the `<parent>` version in `docker/trino/shim/pom.xml`, and the
`docker/trino` defaults
+ (`TRINO_VERSION` in `build_image.sh`, `ARG TRINO_VERSION` in `Dockerfile`).
Re-check SPI-surface-coupled
Review Comment:
`build_image.sh` no longer has a `TRINO_VERSION` literal - it reads
`trino.e2e.version` from the root pom - so this step asks the release manager
to edit something that is not there. Drop it from the list; setting
`trino.e2e.version=NNN` above already moves the image default.
##########
.github/workflows/hudi_trino_e2e.yml:
##########
@@ -62,50 +64,139 @@ jobs:
# hudi-trino at HEAD, assembles the plugin dir via the in-repo shim
# (docker/trino/shim, standing in for the not-yet-released upstream
# trinodb/trino plugin/trino-hudi shim), bakes it into a local
- # apachehudi/hudi-trino_481 image, and runs ITTestTrino* against the
- # spark402 compose stack (the only pair with the trinocoordinator service).
+ # apachehudi/hudi-trino-e2e image on top of the released trino.e2e.version
+ # server, and runs ITTestTrino* against the spark402 compose stack (the
only
+ # pair with the trinocoordinator service).
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5
+ - name: Read Trino pin
+ id: trino-pin
+ run: |
+ set -euo pipefail
+ TRINO_SHA=$(sed -n 's|.*<trino.sha>\(.*\)</trino.sha>.*|\1|p'
pom.xml)
Review Comment:
`sed -n ...p` exits 0 and prints nothing when a property stops matching, and
an empty `sha=` makes the commits API answer for the default branch, so the
gate would report drift and skip every step while `trino-e2e` still reports
green. Fail the step when any of the three is empty, the way
`bootstrap_trino.sh` already does for `trino.version`.
--
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]
