feat(trino): track trinodb/trino master via a pinned SHA - #19642
Merged
Conversation
hudi-trino stops pinning released Trino 481 and compiles against trinodb/trino master at the commit in the new trino.sha root pom property (484-SNAPSHOT at 5b82ec9e7116). Trino publishes no SNAPSHOT artifacts, so scripts/trino/ bootstrap_trino.sh builds the needed modules from source into the local m2; CI restores them from an actions cache keyed on the sha, and the nightly compat job now pre-seeds that cache and proposes pin advances via a bot/trino-pin PR (human-merged) instead of only alarming on drift (apache#19379). SPI fixups 481 -> pin: split SPI rewrite (ConnectorSplitBatch removed, getNextBatch(int, DynamicFilterSnapshot), getSplits with Set<ColumnHandle>, wait timeout via getRequestedDynamicFilterWaitTimeoutMillis), createPageSource gains Optional<ConnectorTableCredentials>, TypeSignature -> TypeDescriptor, CacheKeyProvider returns CacheKey, jts-core is now SPI surface (provided in module and shim), Hive3MinioDataLake -> Hive3FlociDataLake/Floci in tests, dead HudiSplit.getSplitInfo removed. E2E keeps the released server image (new trino.e2e.version property, also feeding trino-jdbc) and auto-skips when the pin has SPI-relevant drift from it; the shim plugin dir is now unzipped from the packaging zip (trino-maven-plugin 24 emits only the zip). Release branches pin back to a released Trino; the release guide gains the pin-back steps.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19642 +/- ##
============================================
+ Coverage 77.70% 77.79% +0.08%
- Complexity 32988 33044 +56
============================================
Files 2524 2525 +1
Lines 139525 139619 +94
Branches 16786 16887 +101
============================================
+ Hits 108424 108619 +195
+ Misses 23475 23382 -93
+ Partials 7626 7618 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
hudi-agent
reviewed
Aug 14, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Appreciate the design proposal! This switches hudi-trino on master to track trinodb/trino master via a pinned SHA — built from source, with io.trino no longer resolving from Maven Central — and pins back to a released Trino at release time. The core motivation (avoiding big-bang, multi-release SPI migrations) is clear and the release-branch pin-back is documented, but a few design areas could use more detail: the reproducibility of SNAPSHOT-keyed local artifacts, the E2E coverage gap during SPI drift windows, the read-path split-source completion-semantics change, and how the version-coupled jts-core scope is handled at pin-back (see inline comments). Once these are addressed, a Hudi committer or PMC member can take it from here for a deeper review.
Member
Author
|
@wombatu-kun Can you please help to review this? We will need this for the 1.2.1 release too. |
wombatu-kun
reviewed
Aug 17, 2026
…he#19640) The pin's blob-cache migration moved cache managers out of catalog properties: fs.cache.directories/max-sizes no longer exist as catalog config, caches come from host-loaded managers, and the cache span is BlobCache.get instead of FileSystemCache.cacheInput. - HudiQueryRunner loads the memory blob cache manager unconditionally, mirroring IcebergQueryRunner; the alluxio tests register the alluxio manager via withBlobCache + AlluxioBlobCachePlugin (TestHudiSmokeTest grows a getBlobCacheProperties hook for the caching smoke subclass). - TestHudiMemoryCacheFileOperations expects BlobCache.get and its span filter includes BlobCache.* spans. - TestTupleDomainUtilsTest wraps varchar values in Slices.utf8Slice: TypeUtils.writeNativeValue no longer coerces raw Strings (482). - New test deps trino-blob-cache-memory/-alluxio (BOM-managed); added to the bootstrap module list; trino-m2 cache key bumped to v2 since the cached artifact set changed. All five affected test classes pass locally (95 tests) on JDK 25 against the pinned 484-SNAPSHOT artifacts.
hudi-agent
reviewed
Aug 17, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the update! This round migrates the Trino connector tests to upstream master's loadable blob-cache plugin model — adding the trino-blob-cache-alluxio/trino-blob-cache-memory test dependencies, wiring MemoryBlobCachePlugin/AlluxioBlobCachePlugin through the HudiQueryRunner builder, splitting cache config into a getBlobCacheProperties() hook, renaming the FileSystemCache.cacheInput spans to BlobCache.get, and switching the VARCHAR multipleValues test values to Slice. This directly addresses @wombatu-kun's "test-hudi-trino-plugin is red — Alluxio cache moved into the loadable blob-cache plugin" comment.
I traced these changes and found no new correctness issues: imports (Map/Optional/Slices) are present, the VARCHAR-to-Slice conversion is applied consistently across all three usages, cacheDirectory is initialized before getBlobCacheProperties() is consumed, and the now-unconditional "memory" blob cache manager stays inert unless a connector flag engages it (verified against the no-cache/memory/alluxio test assertions, which exercise distinct cache subsystems). The withPlugin/withBlobCache builder methods come from upstream Trino's DistributedQueryRunner.Builder, which CI's test-hudi-trino-plugin job will validate.
Note that the earlier design/architecture discussion points remain open and untouched by this round — the SNAPSHOT-keyed local m2 reproducibility, the "alternatives considered"/pin-ownership writeup, the E2E drift-window coverage gap, the HudiSplitSource completion-semantics change, and the jts-core scope re-check at pin-back — as do the other reviewers' workflow, release-guide, and README comments. Please take a look at those still-open inline comments, and this should be ready for a Hudi committer or PMC member to take it from here.
- hudi_trino_compat.yml: split into a read-only build job (runs mvnw from the trino checkout with contents: read) and a propose-pin-advance job that holds the only push credential and runs no third-party code; drop the bot gh pr create (GITHUB_TOKEN PRs start no workflow runs, so required checks would never report) in favor of pushing bot/trino-pin plus a step-summary one-liner for a committer; title type becomes chore(trino); the pin advance now also refreshes trino.e2e.version from the latest released tag. - hudi_trino_e2e.yml: drift gate switched from the compare API (300-file cap, saturated by a single release cycle) to uncapped per-path commit queries; scripts/trino/** added to both paths lists. - release_guide.md: pin-back moved to Cut a release branch, before the source tarball is generated (the voted RC must not ship a -SNAPSHOT trino pin); added an SPI-surface scope re-check (jts-core/482) to the pin-back steps. - hudi-trino/README.md: fast-iteration loop fixed (version read + unzip moved into step 3, which the loop repeats); pin ownership/cadence documented. - TestHudiSmokeTest: new testDynamicFilterEliminatesAllSplits covering the isNone() -> finished early-termination in the rewritten HudiSplitSource. - Root pom: pin comment names the nightly job as the e2e-version owner.
wombatu-kun
reviewed
Aug 17, 2026
- HudiSplitSource.isFinished now reports unfinished while trinoException is pending: the failure callback finishes the queue, and claiming finished at that point let the engine stop polling and end the scan silently instead of surfacing the split-loading failure on the next getNextBatch. - testDynamicFilterEliminatesAllSplits now pins probe-side 'Input: 0 rows' via EXPLAIN ANALYZE; the bare empty-result assert also passed when the filter never engaged and the join simply discarded every row. - e2e drift gate: lib/trino-filesystem dropped (ships inside the plugin dir, cannot skew the boot); the gate keys on the two boundary-crossing surfaces, core/trino-spi and the HdfsFileSystemLoader contract (trino-filesystem-manager + the server image's hdfs jar set). API responses are assigned to a variable before iterating so a transient gh error aborts the step instead of reading error JSON as drift. - build_image.sh derives its default server version from the root pom's trino.e2e.version (the literal default would rot when the bot bumps the pom). - hudi_trino_compat.yml gains a trino_ref dispatch input so the release pin-back can verify and pin the released tag exactly; the guide's step 3 claim now matches what the workflow does.
wombatu-kun
reviewed
Aug 17, 2026
- HudiBackgroundSplitLoader's whenAllComplete combiner surfaces generator failures before finishing the queue: Guava does not order the per-future exception callbacks against the combiner, so a failure in the last future could finish the queue before trinoException was set and isFinished would answer true with no pending exception. - New TestHudiSplitSource unit tests (via a package-private constructor seam that keeps the production error-listener wiring): NONE snapshot terminates the source with undrained splits, a loader failure blocks isFinished and surfaces on the next batch, and normal completion still finishes. - e2e drift gate: lib/trino-filesystem restored and lib/trino-memory-context added -- HdfsClassLoader delegates exactly those packages from the server's hdfs jars back to the plugin's bundled copies, so bundling is why they cross the boundary; comment rewritten accordingly. Read-pin steps in ci and e2e fail loudly on an empty sed result (an empty sha would query the default branch and skip the suite green). - All three trino-m2 cache keys gain the bootstrap-script hash so a module-list change invalidates caches without a pin move. - compat job now also runs the hudi-trino test suite against trino HEAD (test-source and behavioral drift blocked pin advances invisibly before); drift-issue handler covers test failures; JDK 17 leg installs the two extra client modules the tests need. - release guide no longer asks for a build_image.sh literal edit (the script reads trino.e2e.version from the root pom since the round-2 fix).
Contributor
|
@voonhous it seems you forgot to push updates. please answer the threads after you pushed fixes, not before. |
Member
Author
|
Pushed! |
github-actions
Bot
added
size:XL
wombatu-kun
approved these changes
Aug 17, 2026
| 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` |
Contributor
There was a problem hiding this comment.
This git log list is missing lib/trino-memory-context, which the e2e drift gate in hudi_trino_e2e.yml does gate on. Add it here so the guide enumerates the same five boundary-crossing paths.
Member
Author
There was a problem hiding this comment.
Fixed: step 3 now lists the same five paths as the gate, including lib/trino-memory-context.
Collaborator
voonhous
added a commit
to voonhous/hudi
that referenced
this pull request
Aug 18, 2026
* feat(trino): track trinodb/trino master via a pinned SHA (apache#19640) hudi-trino stops pinning released Trino 481 and compiles against trinodb/trino master at the commit in the new trino.sha root pom property (484-SNAPSHOT at 5b82ec9e7116). Trino publishes no SNAPSHOT artifacts, so scripts/trino/ bootstrap_trino.sh builds the needed modules from source into the local m2; CI restores them from an actions cache keyed on the sha, and the nightly compat job now pre-seeds that cache and proposes pin advances via a bot/trino-pin PR (human-merged) instead of only alarming on drift (apache#19379). SPI fixups 481 -> pin: split SPI rewrite (ConnectorSplitBatch removed, getNextBatch(int, DynamicFilterSnapshot), getSplits with Set<ColumnHandle>, wait timeout via getRequestedDynamicFilterWaitTimeoutMillis), createPageSource gains Optional<ConnectorTableCredentials>, TypeSignature -> TypeDescriptor, CacheKeyProvider returns CacheKey, jts-core is now SPI surface (provided in module and shim), Hive3MinioDataLake -> Hive3FlociDataLake/Floci in tests, dead HudiSplit.getSplitInfo removed. E2E keeps the released server image (new trino.e2e.version property, also feeding trino-jdbc) and auto-skips when the pin has SPI-relevant drift from it; the shim plugin dir is now unzipped from the packaging zip (trino-maven-plugin 24 emits only the zip). Release branches pin back to a released Trino; the release guide gains the pin-back steps. * test(trino): adapt cache tests to the blob-cache SPI at the pin (apache#19640) The pin's blob-cache migration moved cache managers out of catalog properties: fs.cache.directories/max-sizes no longer exist as catalog config, caches come from host-loaded managers, and the cache span is BlobCache.get instead of FileSystemCache.cacheInput. - HudiQueryRunner loads the memory blob cache manager unconditionally, mirroring IcebergQueryRunner; the alluxio tests register the alluxio manager via withBlobCache + AlluxioBlobCachePlugin (TestHudiSmokeTest grows a getBlobCacheProperties hook for the caching smoke subclass). - TestHudiMemoryCacheFileOperations expects BlobCache.get and its span filter includes BlobCache.* spans. - TestTupleDomainUtilsTest wraps varchar values in Slices.utf8Slice: TypeUtils.writeNativeValue no longer coerces raw Strings (482). - New test deps trino-blob-cache-memory/-alluxio (BOM-managed); added to the bootstrap module list; trino-m2 cache key bumped to v2 since the cached artifact set changed. All five affected test classes pass locally (95 tests) on JDK 25 against the pinned 484-SNAPSHOT artifacts. * review(19642): address round-1 review - hudi_trino_compat.yml: split into a read-only build job (runs mvnw from the trino checkout with contents: read) and a propose-pin-advance job that holds the only push credential and runs no third-party code; drop the bot gh pr create (GITHUB_TOKEN PRs start no workflow runs, so required checks would never report) in favor of pushing bot/trino-pin plus a step-summary one-liner for a committer; title type becomes chore(trino); the pin advance now also refreshes trino.e2e.version from the latest released tag. - hudi_trino_e2e.yml: drift gate switched from the compare API (300-file cap, saturated by a single release cycle) to uncapped per-path commit queries; scripts/trino/** added to both paths lists. - release_guide.md: pin-back moved to Cut a release branch, before the source tarball is generated (the voted RC must not ship a -SNAPSHOT trino pin); added an SPI-surface scope re-check (jts-core/482) to the pin-back steps. - hudi-trino/README.md: fast-iteration loop fixed (version read + unzip moved into step 3, which the loop repeats); pin ownership/cadence documented. - TestHudiSmokeTest: new testDynamicFilterEliminatesAllSplits covering the isNone() -> finished early-termination in the rewritten HudiSplitSource. - Root pom: pin comment names the nightly job as the e2e-version owner. * review(19642): address round-2 review - HudiSplitSource.isFinished now reports unfinished while trinoException is pending: the failure callback finishes the queue, and claiming finished at that point let the engine stop polling and end the scan silently instead of surfacing the split-loading failure on the next getNextBatch. - testDynamicFilterEliminatesAllSplits now pins probe-side 'Input: 0 rows' via EXPLAIN ANALYZE; the bare empty-result assert also passed when the filter never engaged and the join simply discarded every row. - e2e drift gate: lib/trino-filesystem dropped (ships inside the plugin dir, cannot skew the boot); the gate keys on the two boundary-crossing surfaces, core/trino-spi and the HdfsFileSystemLoader contract (trino-filesystem-manager + the server image's hdfs jar set). API responses are assigned to a variable before iterating so a transient gh error aborts the step instead of reading error JSON as drift. - build_image.sh derives its default server version from the root pom's trino.e2e.version (the literal default would rot when the bot bumps the pom). - hudi_trino_compat.yml gains a trino_ref dispatch input so the release pin-back can verify and pin the released tag exactly; the guide's step 3 claim now matches what the workflow does. * review(19642): address round-3 review - HudiBackgroundSplitLoader's whenAllComplete combiner surfaces generator failures before finishing the queue: Guava does not order the per-future exception callbacks against the combiner, so a failure in the last future could finish the queue before trinoException was set and isFinished would answer true with no pending exception. - New TestHudiSplitSource unit tests (via a package-private constructor seam that keeps the production error-listener wiring): NONE snapshot terminates the source with undrained splits, a loader failure blocks isFinished and surfaces on the next batch, and normal completion still finishes. - e2e drift gate: lib/trino-filesystem restored and lib/trino-memory-context added -- HdfsClassLoader delegates exactly those packages from the server's hdfs jars back to the plugin's bundled copies, so bundling is why they cross the boundary; comment rewritten accordingly. Read-pin steps in ci and e2e fail loudly on an empty sed result (an empty sha would query the default branch and skip the suite green). - All three trino-m2 cache keys gain the bootstrap-script hash so a module-list change invalidates caches without a pin move. - compat job now also runs the hudi-trino test suite against trino HEAD (test-source and behavioral drift blocked pin advances invisibly before); drift-issue handler covers test failures; JDK 17 leg installs the two extra client modules the tests need. - release guide no longer asks for a build_image.sh literal edit (the script reads trino.e2e.version from the root pom since the round-2 fix). * review(19642): sync guide pin-back paths with the e2e drift gate (cherry picked from commit 541ed1c)
3 tasks
voonhous
added a commit
to voonhous/hudi
that referenced
this pull request
Sep 8, 2026
The pin-back (8c12d47) moved trino.version from the 484-SNAPSHOT master pin to 483 but left HudiCacheKeyProvider on the post-483 cache SPI, so hudi-trino stopped compiling: package io.trino.spi.cache does not exist cannot find symbol: class CacheKey cannot find symbol: method locationKey(io.trino.filesystem.Location) At 483 CacheKeyProvider.getCacheKey returns Optional<String> and the interface has no locationKey; io.trino.spi.cache has no files at that tag. Restore the file to its pre-apache#19642 form, which is the 483 contract. The pin-back scoped its SPI audit to io.trino.blob.cache, whose five importers are all under src/test. This file imports io.trino.spi.cache instead and rides a signature change within io.trino.filesystem.cache, so that grep could not see it. Re-checked every io.trino import in hudi-trino/src against the 483 tag: CacheKey was the only one missing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the issue this Pull Request addresses
Closes #19640.
hudi-trinopins Trino 481 while Trino is at 483, and the nightly SPI drift job is red against trino master (#19379). Pin-to-pin upgrades arrive as multi-release big-bang migrations.Summary and Changelog
Master now tracks trinodb/trino master at a pinned commit (new
trino.sha+trino.version=484-SNAPSHOTin the root pom), built from source since Trino publishes no SNAPSHOT artifacts. Releases still pin a released Trino; the release guide gains the pin-back steps.scripts/trino/bootstrap_trino.shinstalls the neededio.trinomodules (incl. the unpublished test-jars) into the local m2; contributors and CI run the same script.trino.sha; the nightly compat job pre-seeds the cache and opens a human-mergedbot/trino-pinPR when trino HEAD compiles, still filing the drift issue when it does not.trino.e2e.version, also feedstrino-jdbc) and auto-skips with a notice when the pin has SPI drift against it.DynamicFilterSnapshot,ConnectorSplitBatchremoved),createPageSourcegains the credentials arg,TypeSignature->TypeDescriptor,CacheKeyProviderreturnsCacheKey,jts-corenowprovided(SPI surface), Minio -> Floci test containers.Impact
No public API or storage-format change. On master,
io.trinono longer resolves from Maven Central: run the bootstrap script once per pin advance (see the module README). E2E coverage pauses during drift windows.Risk Level
Medium: the split-source rewrite changes completion semantics. Mitigated by mirroring upstream's migrated connector; compile, test-compile, javadoc and the docker shim build are all green locally against the pin, and this PR's CI exercises the new workflows.
Documentation Update
hudi-trino/README.md(bootstrap-first build),docker/README.md,release/release_guide.md(pin-back subsection). No config or website changes.Contributor's checklist