fix(trino): read uncompacted MDT HFILE log deltas and guard index pruning - #19298
Merged
Conversation
voonhous
changed the title
voonhous
marked this pull request as draft
July 15, 2026 09:03
voonhous
force-pushed
the
19279--mdt-hfile-log-deltas
branch
from
July 15, 2026 09:12
545ca14 to
6d3bb86
Compare
3 tasks
voonhous
force-pushed
the
19279--mdt-hfile-log-deltas
branch
from
July 15, 2026 16:49
6d3bb86 to
b1352ed
Compare
…ning Fixes apache#19279: with the connector default hudi.metadata-enabled=true, any query on a table whose metadata table (MDT) has uncompacted delta commits failed with 'Native HFILE log files are not supported by the Hudi Trino connector'. Since MDT compaction fires only every hoodie.metadata.compact.max.delta.commits (default 10) deltas, actively written tables spend most of their life in that state; the E2E testcontainers pipeline had to set hudi.metadata-enabled=false to pass. - HudiTrinoIOFactory.getFileFormatUtils now returns hudi-common's HFileUtils for HFILE. The native-log read path (HoodieNativeLogFileReader) needs only its readFooter, and HFileUtils is hadoop-free (it decodes via hudi-io's native HFile reader), so it is safe in the Trino runtime -- unlike parquet, whose ParquetUtils lives in the hadoop-excluded hudi-hadoop-common and needs the Trino-native HudiTrinoParquetFileFormatUtils. The reader-factory side (newHFileFileReader) was already implemented; it is how compacted MDT base HFiles are read today. MDT reads never touch HudiTrinoReaderContext. - Guard the partition-stats pruning call in HudiBackgroundSplitLoader.getPartitionInfos: any pruning failure now logs and proceeds unpruned, mirroring the metastore fallback the MDT partition listing directly above it already has. Index pruning is an optimization and must never fail the query. The per-file-slice indexes (column-stats, record-level, secondary) already swallow failures via their future+catch pattern. - Guard the MDT-backed file-system-view load in HudiSnapshotDirectoryLister: on failure, close the partial view and fall back to a direct-listing view built over an MDT-disabled table metadata. This was the second unguarded synchronous MDT read. Tests: UncompactedMetadataHudiTablesInitializer writes a partitioned COW table with an ENABLED metadata table (column-stats + partition-stats indexes) and compact.max.delta.commits=100 -- the inverse of the zip fixtures' compact.max.delta.commits=1, whose always-compacted MDTs are why nothing caught this -- leaving native HFILE log deltas in the files/column_stats/ partition_stats MDT partitions. MDT HFILE writing is native (hudi-io HFileWriterImpl); the 'requires hbase' note in older initializers is stale. TestHudiUncompactedMetadataTable covers MDT-backed listing, MDT-on vs MDT-off result equality, partition-stats pruning (the exact crash path) and column-stats file skipping over the uncompacted stats. The E2E follow-up (separate branch trino-e2e-testcontainers): drop the hudi.metadata-enabled=false workaround from the Trino catalog so the pipeline runs MDT-on.
voonhous
force-pushed
the
19279--mdt-hfile-log-deltas
branch
from
July 31, 2026 10:28
b1352ed to
8ddb323
Compare
voonhous
marked this pull request as ready for review
July 31, 2026 10:28
Member
Author
|
@wombatu-kun This is ready for review, thank you! |
github-actions
Bot
added
size:L
hudi-agent
reviewed
Jul 31, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the contribution! This PR lets the Trino connector read uncompacted metadata-table HFILE log deltas (by returning the hadoop-free HFileUtils for HFILE) and hardens two synchronous MDT reads — partition-stats pruning and the MDT-backed file-system-view load — to degrade gracefully instead of failing the query. I traced the native-log read path (only readFooter is exercised, and it decodes via hudi-io's native reader), confirmed the prunePartitionsSafely rewrite is behavior-preserving and fails safe (over-read, never drops data), and checked the shared lazyTableMetadata lifecycle around the new fallback (partition listing reads it before the fallback fires, index skipping stays correct over the direct-listed file set, and nothing double-closes it). No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. A couple of minor naming nits below.
cc @yihua
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19298 +/- ##
============================================
+ Coverage 75.55% 75.71% +0.15%
- Complexity 32657 32789 +132
============================================
Files 2574 2576 +2
Lines 142995 143558 +563
Branches 17530 17607 +77
============================================
+ Hits 108038 108691 +653
+ Misses 26922 26856 -66
+ Partials 8035 8011 -24
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The private computeScalarWith helper duplicated AbstractTestQueryFramework's existing computeScalar(Session, String). Drop it and call the inherited one.
wombatu-kun
reviewed
Jul 31, 2026
…allbacks Review follow-ups on the HudiSnapshotDirectoryLister fallback and its tests: - Drop fileSystemView.close() in the catch: closing the failed view would also close the shared HoodieTableMetadata behind lazyTableMetadata, which the split loader and the index supports still read through. Nothing else closes that instance today. - Build the direct-listing fallback via HoodieTableFileSystemView.fileListingBasedFileSystemView with the same completed-commits timeline HudiUtil.getFileSystemView uses, instead of the MDT-disabled NativeTableMetadataFactory detour. - Pin both MDT-failure fallbacks: the initializer writes a corrupted twin table (hudi_corrupted_mdt_pt_cow) whose MDT log files are damaged before upload, and new tests assert the direct-listing fallback returns complete rows and prunePartitionsSafely degrades to a full scan. The damage targets the HFile trailer fields inside the block content and leaves the log block framing intact: framing damage is classified as a HoodieCorruptBlock and skipped without an exception, and bytes near EOF are only trailer padding (hudi-io's 4096-byte trailer keeps its magic/protobuf at the start). - Assert exact split counts in the partition-stats pruning test: equal to a metastore-pruned part_col scan of p1 and strictly below the full scan. Not a literal count, since file groups per partition depend on the write client's small-file packing for this runtime-written table. - Reuse AbstractMergerHudiTablesInitializer.HUDI_META_COLUMNS (now package-private) instead of redeclaring the five _hoodie_* columns.
…s get source fields The exact split-count assertion added in the previous commit exposed that partition-stats pruning never ran on the fixture table: the CI log shows 'Partition stats index definition is missing or has no source fields defined' and both partitions being listed. Root cause: the MDT bootstrapped during commit 1 on an empty table, so ColumnStatsIndexer.postInitialization registered the column_stats index definition with an empty source-field list, and HoodieJavaWriteClient.updateColumnsToIndexWithColStats is a no-op (unlike the Spark client, which refreshes the definition on each commit), so the definition stayed empty forever and the connector's canApply() silently rejected the col-stats and partition-stats indexes. The pruning and file-skipping tests -- clean and corrupted -- were passing vacuously. Write commit 1 with the MDT disabled and open a second write client with the MDT (and stats indexes) enabled for commits 2 and 3: the bootstrap then sees commit 1's files and registers the definition with real source fields. Commits 2 and 3 still leave uncompacted HFILE log deltas in the files/ column_stats/partition_stats MDT partitions, so the original regression coverage is unchanged, and the corrupted twin still fails every MDT read through its damaged log deltas.
wombatu-kun
reviewed
Jul 31, 2026
…ng by split count Address review comments on apache#19298: - The initializer now opens a metadata-table reader over the corrupted twin after damaging the log files and asserts getAllPartitionPaths throws, so the fallback tests cannot pass vacuously if the fixed trailer offsets ever miss (checkState(corrupted > 0) only proved bytes were flipped). - testColumnStatsFileSkippingOverUncompactedStats raises the col-stats wait timeout to 10s (shouldSkipFileSlice keeps the file on any failure or after the 1s default timeout, so the old value-only assertion passed without the deltas ever being read) and asserts the filtered scan uses strictly fewer splits than the full scan.
wombatu-kun
reviewed
Jul 31, 2026
…ion fixture Review follow-ups on the uncompacted-MDT fixture: - The javadoc claimed the zip fixtures escaped this bug because their MDTs are 'always freshly compacted'. Wrong on both counts: only some fixtures compact after every commit, and hudi_trips_cow_v8 (queried MDT-on in TestHudiSmokeTest) ships a 14KB uncompacted delta in the MDT files partition. The real difference is the log FORMAT: those fixtures predate the native-log write path, so their deltas are #HUDI# block-format logs carrying HFILE_DATA_BLOCKs (block type ordinal 4 in the header), which the connector already read through its HFile content reader. Only whole-file native HFILE log files (*.log.hfile, selected by filename in FSUtils.isNativeLogFile) go through HoodieNativeLogFileReader and the previously unimplemented getFileFormatUtils(HFILE) path. Reworded the initializer and test javadocs (and the PR description) accordingly, and corrected the corruption helper's comment, which described block framing these native files do not have. - corruptMetadataLogFiles now requires at least one corrupted log file in EVERY MDT partition directory, not one overall, so small deltas in one partition cannot silently survive; the walk also excludes anything not directly inside a partition directory. - verifyMetadataTableUnreadable catches HoodieMetadataException only -- the wrapper BaseTableMetadata puts around a genuine read failure -- so the bare IllegalArgumentException thrown for a never-initialized MDT fails initialization instead of masquerading as corruption. - The MDT-off-first-commit workaround now cites HUDI-8801 (the Java client's no-op updateColumnsToIndexWithColStats) so it can be dropped when that lands.
Narrowing the catch to HoodieMetadataException left the checked exceptions previously swallowed by catch (Exception) unreported: IOException declared by getAllPartitionPaths and Exception from the implicit close() of the try-with-resources. Declare throws Exception so anything unexpected fails table initialization loudly instead of being swallowed; the caller already declares it.
The createWriteClient comment still repeated the debunked claim that the zip fixtures always compact after every commit; the class javadoc carries the corrected native-log-format rationale, so the file now tells one story.
hudi-agent
reviewed
Aug 1, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this! The PR makes the Trino connector read uncompacted MDT native HFILE log deltas (via hudi-common's hadoop-free HFileUtils) and adds fallbacks so a metadata-table read failure degrades to direct file listing / unpruned split generation instead of failing the query. I traced the three production changes — the HFILE getFileFormatUtils path (only readFooter is needed and it's implemented), the HudiSnapshotDirectoryLister fallback (preserves result correctness and uses the same completed-commits timeline as the existing helper), and the prunePartitionsSafely refactor (behavior-preserving vs. the prior Optional chain, with the guard only degrading to no-pruning) — and each MDT read path that could fail the query is guarded. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
cc @yihua
wombatu-kun
approved these changes
Aug 1, 2026
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.
Stacked on #19295 (-> #19288 -> #18837) -- only the head commit is new here.
Describe the issue this Pull Request addresses
Closes #19279
With the connector default
hudi.metadata-enabled=true, any query on a table whose metadata table (MDT) has uncompacted delta commits fails with "Native HFILE log files are not supported by the Hudi Trino connector". MDT compaction only runs every 10 delta commits by default, so an actively written table spends most of its life in that state -- default Spark writer plus default catalog config meant every read failed. The E2E pipeline currently setshudi.metadata-enabled=falseto work around it.Summary and Changelog
The gap turned out to be small: the connector already reads HFile base files (compacted MDT) through hudi-io's native reader, and the native-log read path only needs
getFileFormatUtils(HFILE).readFooter. hudi-common'sHFileUtilsis hadoop-free (it decodes via hudi-io), so unlike parquet there is no need for a Trino-native implementation.HudiTrinoIOFactory.getFileFormatUtilsnow returnsHFileUtilsfor HFILE, making uncompacted MDT deltas (native HFILE log files) readable.HudiBackgroundSplitLoaderno longer fails the query: on error it logs and proceeds unpruned, same as the metastore fallback the partition listing right above it already had. This was the unguarded path in the issue's stack.HudiSnapshotDirectoryLister(the other unguarded synchronous MDT read) falls back to direct file listing on error.Tests: a new initializer writes two partitioned COW tables with the MDT enabled and
compact.max.delta.commits=100, leaving real native HFILE log files (*.log.hfile, the whole file is an HFile) in thefiles/column_stats/partition_statsMDT partitions. The zip fixtures could not catch this even where their MDTs are uncompacted: they predate the native-log write path, so their MDT deltas are block-format logs carrying HFILE data blocks, which the connector already read through its HFile content reader. The second table is a corrupted twin: its MDT log deltas are damaged in place (targeting the HFile trailer fields, in every MDT partition, so any read throws), and the initializer asserts that a metadata-table read of it actually fails before uploading.TestHudiUncompactedMetadataTablecovers, on the clean table: MDT-backed listing, MDT on/off result equality, partition-stats pruning with split-count assertions (the exact crash from the issue) and col-stats file skipping with split-count assertions (col-stats wait timeout raised so the skip decision actually consumes the stats); and on the corrupted twin, the two fallbacks: direct file listing when the MDT-backed file-system-view load throws, and unpruned split generation when the partition-stats read throws. Side note: MDT writing needs no hbase anymore (hudi-io'sHFileWriterImpl), so the "requires hbase" comment in the older initializers is stale.Impact
Tables with uncompacted MDTs are readable under default configs, and index pruning / MDT listing degrade gracefully instead of failing queries. No API or config changes. Follow-up on the E2E branch: drop the
hudi.metadata-enabled=falsecatalog workaround so the pipeline runs MDT-on.Risk Level
low -- the format support is a one-line dispatch to existing hudi-common/hudi-io code, and the two guards mirror fallback patterns already in the connector; all pinned by the new tests.
Documentation Update
none
Contributor's checklist