[DNM] Release 1.2.1 staging - #19557
Open
voonhous wants to merge 255 commits into
Open
Conversation
… path (FLINK-35702) (apache#18700) (cherry picked from commit 47bf4e4)
…link 2.1 Dremel path (FLINK-35702) (apache#18701) * refactor(flink): Remove legacy Parquet nested readers superseded by Flink 2.1 Dremel path (FLINK-35702) * Fix flaky IT test (cherry picked from commit 004d159)
…ache#18394) (cherry picked from commit f8e4b9f)
…erformance (apache#17517) * perf: Reduce unnecessary `FSDataOutputStream#hsync` to enhance append performance 1. Reduce unnecessary `FSDataOutputStream#hsync` to enhance append performance Signed-off-by: TheR1sing3un <chaoyang@apache.org> * feat: flush behavior compatible with the block append mode 1. flush behavior compatible with the block append mode Signed-off-by: TheR1sing3un <chaoyang@apache.org> * fixup: address review - drop syncDuringFlush, expose explicit sync() Following @danny0405's suggestion in the PR review, ensure only commit-level visibility on the production path: - Remove the `withSyncDuringFlush` builder option and the `flush(boolean)` overload on HoodieLogFormatWriter; the production path no longer flushes or hsyncs at appendBlocks. - Expose `Writer#sync()` (flush + hsync) as an explicit API for tests that assert per-append visibility on the underlying file system. - closeStream still calls sync() once before close so a closed writer guarantees data is persisted to DataNodes. - Update tests that previously relied on `withSyncDuringFlush(true)` to call `writer.sync()` explicitly before per-append FileStatus size assertions, and rename the related assertion message to drop the misleading "auto-flushed" wording. --------- Signed-off-by: TheR1sing3un <chaoyang@apache.org> (cherry picked from commit 1fffd70)
) PR apache#18394 added hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/muttley/README.md without an Apache license header, causing apache-rat:check to fail on every new build of master ("Too many files with unapproved license: 1"). Prepend the standard Apache 2.0 HTML-comment license header so RAT passes. Verified locally: cd hudi-flink-datasource/hudi-flink mvn -Pflink1.20 -Dscala-2.12 apache-rat:check -> Rat check: Summary over all files. Unapproved: 0 Co-authored-by: Xinli Shang <shangxinli@apache.org> (cherry picked from commit f2f6203)
* Add variant type adapter for Flink * address the review comments (cherry picked from commit 5e72c96)
* chore: migrate the flink ITs to run with flink2.1 * fix compile errors * fix test failures * fix test failure * address review comments (cherry picked from commit b6bc165)
…alidation - Phase 3 (apache#18405) * feat: Add Spark streamer validators for phase 3 precommit validation Implements phase 3 of the precommit validation framework by adding: - SparkKafkaOffsetValidator: Validates Kafka offset consistency - SparkValidationContext: Provides Spark-specific validation context - SparkStreamerValidatorUtils: Utility functions for Spark streamer validation - Comprehensive test coverage for all validator components - Integration with StreamSync and HoodiePreCommitValidatorConfig Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review and fix checkstyle violations - Remove unused imports (java.io.IOException, HoodieCommitMetadata, HoodieTestTable, Option) that caused checkstyle build failures - Remove accidentally committed bootstrap_register_only_issue.md - Cache writeStatusRDD before collect() to prevent second DAG evaluation and potential driver OOM - Add comment explaining why validator runs before writeClient.commit(): offset validation is a stronger guard than commitOnErrors and must prevent the commit when data loss is detected - Clarify buildCommitMetadata() produces a pre-commit preview object, not a fully-constructed commit record - Add Javadoc to SparkKafkaOffsetValidator and SparkStreamerValidatorUtils explaining incompatibility with SparkValidatorUtils (different interface and constructor signature) to prevent misconfiguration - Add two-commit integration tests (testSecondCommitMatchingOffsetsPasses, testSecondCommitDataLossDetected) using HoodieTestTable to exercise the real offset comparison path, not just the first-commit skip path * fix: skip non-SparkPreCommitValidator classes in SparkValidatorUtils SparkKafkaOffsetValidator (and similar streaming validators) extend BasePreCommitValidator with a (TypedProperties) constructor, not the (HoodieSparkTable, HoodieEngineContext, HoodieWriteConfig) constructor that SparkValidatorUtils expects. Listing such a validator in hoodie.precommit.validators previously caused a reflection error in the Spark table write path. Add a Class.isAssignableFrom check to filter out classes that don't implement SparkPreCommitValidator before attempting instantiation, with a clear warning pointing users to SparkStreamerValidatorUtils for streaming validators. * ci: trigger CI re-run for flaky trino test * fix: address reviewer comments on pre-commit streaming offset validator - Unpersist cached RDD in finally block to prevent executor memory leak - Let IOException propagate from loadPreviousCommitMetadata instead of silently swallowing it - Filter empty validator class names before Class.forName to handle trailing comma in config - Add write error count to validation message to distinguish write failures from silent data loss * fix: address reviewer follow-up comments on pre-commit streaming validator - Change runValidators to accept List<WriteStatus> instead of JavaRDD to fix RDD unpersist-before-commit bug; StreamSync now caches the RDD, collects to list for validators, passes RDD to commit, then unpersists - Remove generic catch(Exception) in loadPreviousCommitMetadata so non-IOException failures propagate instead of silently skipping validation - Implement getPreviousCommitInstant() in SparkValidationContext via timeline lookup instead of throwing UnsupportedOperationException - Add Objects::nonNull filter when building writeStats list - Add BasePreCommitValidator assignability guard in SparkStreamerValidatorUtils to warn and skip SparkPreCommitValidator classes (reverse-direction guard) - Eliminate double class loading in SparkValidatorUtils by combining filter+map into a single flatMap; remove unused ReflectionUtils import - Remove trivial constructor Javadoc from SparkKafkaOffsetValidator - Add HoodieTestUtils import in test; remove Spark context boilerplate now that runValidators accepts List<WriteStatus> directly * fix: guard cache() call when writeStatusRDD is already persisted Calling cache() on an RDD that already has a storage level assigned throws SparkUnsupportedOperationException. The write path may cache the RDD internally before returning it. Track whether we own the cache and only call cache()/unpersist() when the RDD was not already persisted. * Ensure writeStatusRDD is always unpersisted via try/finally * fix: use proper import for StorageLevel instead of fully-qualified class reference 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address final reviewer feedback on pre-commit streaming validator - Move runValidators() inside the try/finally so writeStatusRDD.unpersist() always runs, including on validator exceptions (FAIL policy or HoodieIOException from loadPreviousCommitMetadata). - Use ReflectionUtils.loadClass in SparkValidatorUtils for instantiation, matching SparkStreamerValidatorUtils and the rest of the codebase. - Rename weOwnCache to shouldUnpersist to read in the direction of its actual use (gating unpersist in finally). * fix: only cache writeStatusRDD when pre-commit validators are configured Address danny0405's review comment on PR apache#18405: skip the .cache()/.unpersist() cycle when no pre-commit validators are configured, since without validators the RDD is consumed exactly once by writeClient.commit() and caching adds no value. Guards both the cache call and the validator collect+run on a single validatorsConfigured boolean derived from hoodie.precommit.validators. * address codope review: V2-then-V1 checkpoint key resolution + V2 test coverage Comment 1 (SparkKafkaOffsetValidator hardcoded V1 key): - StreamingOffsetValidator base class now exposes a no-key constructor that auto-resolves the checkpoint via CheckpointUtils.getCheckpoint(metadata), which prefers V2 and falls back to V1. The explicit-key constructor stays for subclasses that read a custom non-streamer key (e.g. Flink's HOODIE_METADATA_KEY). - SparkKafkaOffsetValidator switches to the no-key constructor. Comment 2 (tests only cover V1 path): - testSecondCommitMatchingOffsetsPasses and testSecondCommitDataLossDetected are now parameterized over both V1 and V2 checkpoint keys. - Added testV2CheckpointKeyOnTableVersionEightFires on a HoodieTableVersion.EIGHT table with V2 keys, asserting the validator fires on data loss. --------- Co-authored-by: Xinli Shang <shangxinli@apache.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit b934633)
Corrects a handful of long-standing typos in code comments and test assertion messages across the codebase. No functional changes. - atleast -> at least (8 occurrences) - commited -> committed (1 occurrence in comment) - existance -> existence (1 occurrence in exception message) - transfering -> transferring (2 occurrences) - succesfully -> successfully (1 occurrence in comment) Co-authored-by: Xinli Shang <shangxinli@apache.org> (cherry picked from commit cd2c8b8)
…sion for unshredded variant (apache#18539) * feat(flink): write/read unshredded variant to Flink parquet file writers/readers using Flink's Variant type --------- Co-authored-by: Krishen Bhan <“bkrishen@uber.com”> Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 71aa121)
* feat(spark): add restore_to_instant stored procedure Adds a Spark SQL stored procedure that performs a point-in-time table restore to any instant on the active timeline, with optional post-restore file-existence audit. Unlike rollback_to_savepoint, no savepoint is required at the target instant. Centralizes the MDT pre-check that was inlined in restoreToSavepoint into a new BaseHoodieWriteClient.shouldDeleteMdtBeforeRestore helper, and extends restoreToInstant to invoke it. The helper also catches the penultimate-compaction case (target at or before the second-most-recent MDT compaction) which the previous restoreToSavepoint inline check missed. IO/permission failures now surface as HoodieException instead of being silently swallowed. The audit returns a tri-state result (PASSED / FAILED / INCONCLUSIVE) so transient cloud-storage timeouts are distinguishable from real audit failures; an audit_only mode lets users re-audit a previously completed restore by passing its restore_instant_time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(spark): address PR review: public MDT guard, variable rename, test coverage Add a public deleteMetadataTableIfNecessaryBeforeRestore method to BaseHoodieWriteClient so callers that drive restore via restoreToInstant directly (e.g. the restore_to_instant procedure) can pre-check and pre-emptively delete the MDT before calling restoreToInstant. This closes the coverage gap identified in review: the procedure previously had no MDT protection after shouldDeleteMdtBeforeRestore was removed from restoreToInstant. The procedure now calls client.deleteMetadataTableIfNecessaryBeforeRestore before restoreToInstant and passes the returned boolean as initialMetadataTableIfNecessary, ensuring the penultimate/oldest compaction and timeline-start checks fire on the procedure path. Rename internal Scala variable restoreInstantTime -> startRestoreTimeArg for consistency with the start_restore_time parameter name. Strengthen testRestoreToInstantSkipsMdtCheckWhenMetadataDisabled: the test now verifies (a) deleteMetadataTableIfNecessaryBeforeRestore returns false and deletes the MDT for a target at/before the oldest compaction, and (b) restoreToInstant(target, false) then proceeds without invoking the guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(spark): address PR review round 2 — remove penultimate check, rename public API Remove the penultimate-compaction check from shouldDeleteMdtBeforeRestore. The check fired too aggressively: it deleted the MDT even when the MDT could restore successfully, and then the fresh re-bootstrap failed for record_index-enabled tables (testRLIWithMDTCleaning) or triggered an inline MDT compaction during bootstrap that broke the isMetadataTableRecreatedDuringRestore detection (testRestoreToSavepointDeletesMdtWhenTargetIsBeforePenultimateCompaction). Only the oldest-compaction and timeline-start checks remain, matching the original pre-PR behaviour. Add a catch clause for HoodieException (e.g. TableNotFoundException from a partially initialized MDT directory) so a corrupt-but-present MDT is treated as absent rather than hard-failing the restore. Rename deleteMetadataTableIfNecessaryBeforeRestore -> deleteMdtIfNecessaryBeforeRestore and flip the return value so that true = MDT was deleted (callers negate with ! when passing to restoreToInstant). Update RestoreToInstantProcedure and the corresponding Java test accordingly. Restore the @deprecated annotation on the two-arg rollback overload that was accidentally dropped in a prior commit. Remove testRestoreToSavepointDeletesMdtWhenTargetIsBeforePenultimateCompaction which tested the now-removed penultimate check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: mahsoode <mahsoode@uber.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit e406e5d)
apache#18709) * feat(spark): add show_inflight_commits and cleanup_stale_inflight_commits stored procedures Two new CALL procedures so operators can inspect and remediate stale inflight commits via SQL instead of using hudi-cli. show_inflight_commits(table, min_age_minutes?) lists REQUESTED+INFLIGHT instants from the active timeline. cleanup_stale_inflight_commits(table, allowed_inflight_interval_minutes?, include_ingestion_commits?, dry_run?) rolls back stale write-timeline inflights older than the threshold (default 180 min). COMPACTION, LOG_COMPACTION, and CLUSTERING route to their dedicated table.rollbackInflight* methods (HoodieSparkTable is lazy-init on first such instant); other write actions go through client.rollback(). include_ingestion_commits and dry_run both default to false; dry_run emits rollback_status=NULL and skips write-client construction. A single-method utility HoodieTimelineCleanupUtil (inflightWriteCommitsOlderThan) is added in hudi-spark-common. Tested with 4 show + 9 cleanup unit tests covering empty/threshold/ ingestion-gating/dry_run paths plus COMPACTION, CLUSTERING, partitioned COW, and MOR delta_commit. checkstyle + scalastyle clean. * address review comments on show_inflight_commits / cleanup_stale_inflight_commits - Rename HoodieTimelineCleanupUtil.inflightWriteCommitsOlderThan param `mins` -> `ageMinutes` for clarity. - Replace Duration.ofMinutes(...).getSeconds() * 1000 with .toMillis() in HoodieTimelineCleanupUtil and ShowInflightCommitsProcedure. - Add inflight-state recheck in CleanupStaleInflightCommitsProcedure before client.rollback(): reloads the active timeline and skips the rollback if the instant is no longer INFLIGHT/REQUESTED, preventing destructive rollback of a commit that completed concurrently after detection. --------- Co-authored-by: mahsoode <mahsoode@uber.com> (cherry picked from commit d94b2e2)
…ion (apache#18796) (cherry picked from commit a687786)
…ache#13875) make the HoodieWriteConfig.TABLE_SERVICES_ENABLED effective for Flink. --------- Co-authored-by: fhan <yfhanfei@jd.com> Co-authored-by: danny0405 <yuzhao.cyz@gmail.com> (cherry picked from commit 50eb95c)
…to hudi-flink1.19.x (FLINK-35702) (apache#18809) * feat(flink): Backport Flink 2.1 Dremel nested Parquet reader rewrite to hudi-flink1.19.x (FLINK-35702) (cherry picked from commit 06dc09b)
…ache#18815) (cherry picked from commit 7e5b535)
(cherry picked from commit 62ad88a)
* fix(spark): Add options for archive procedure * set 'enable_metadata' default value to true * fix args in SparkMain * fix options in ArchiveCommitsProcedure * fix(spark): set named parameters with higher priority and improve extractOptions() * optimize entire impl and add UTs for HoodieCLIUtils * optimize ArchiveCommitsProcedure. * optimize ArchiveExecutorUtils and HoodieCLIUtils according to hudi-agent review results. --------- Co-authored-by: fhan <yfhanfei@jd.com> (cherry picked from commit b5c5801)
(cherry picked from commit 76ddebc)
…pache#18371) --------- Co-authored-by: Lokesh Jain <ljain@Lokeshs-MacBook-Pro.local> (cherry picked from commit 516d9e2)
(cherry picked from commit ce29423)
…pache#18782) (cherry picked from commit e5ae111)
(cherry picked from commit 1bf6b44)
…pache#18873) (cherry picked from commit 588998f)
(cherry picked from commit ff72186)
… pass (apache#18871) * perf(streamer): fold validate() error-table WriteStatus sums into one pass (cherry picked from commit 7af8cdf)
…ir (apache#18883) Co-authored-by: fhan <yfhanfei@jd.com> (cherry picked from commit 6e40cff)
Co-authored-by: fhan <yfhanfei@jd.com> (cherry picked from commit da18995)
* fix(flink): fix data loss in stream read from earliest * fix(flink): optimize UTs and refine de-duplicate full-table-scan timeline comment --------- Co-authored-by: fhan <yfhanfei@jd.com> (cherry picked from commit 4e5034d)
… server (apache#19444) (cherry picked from commit 636e6fa)
…-aws is absent (apache#19418) The CloudWatch reporter lives in the optional hudi-aws module and is loaded reflectively, but not every engine bundle shades that module. Selecting hoodie.metrics.reporter.type=CLOUDWATCH without it on the classpath failed with "Unable to load class", naming neither the missing class nor a remedy. Report the missing class, point at hudi-aws-bundle, and name the config to change instead. Other reflection failures are rethrown unchanged. ReflectionUtils.getClass now names the class in the generic failure too, which improves the message for every reflective load in Hudi. Closes apache#15293 (cherry picked from commit 637996c) Cherry-pick adaptation: - Import conflict only. This branch has not taken the hudi-common package reorganization (apache#19195), so HoodieMetricsConfig is still imported from org.apache.hudi.config.metrics rather than org.apache.hudi.common.config.metrics. Kept the release import and added the new VisibleForTesting import alongside it. org.apache.hudi.common.util.VisibleForTesting resolves unchanged here; the class sits in hudi-common rather than hudi-io but has the same fully-qualified name. All three files otherwise apply byte-identically to the upstream delta.
…ime (apache#19452) * fix(timeline): do not NPE on archived instants without a completion time Upgrading a table written by 0.x fails while polling the archived timeline: java.lang.NullPointerException: Cannot invoke "Object.toString()" because the return value of "org.apache.avro.generic.GenericRecord.get(String)" is null at CompletionTimeQueryViewV2.readCompletionTime completionTime is declared ["null","string"] with a null default in HoodieLSMTimelineInstant, and instants archived before that field existed carry no value for it. setCompletionTime already handles the null case by falling back to the instant time, with a comment saying so, but readCompletionTime called toString() on the raw field before reaching it. Read the field as an Object and let the existing fallback apply. Adds unit tests for both the missing and present cases; readCompletionTime is widened to package-private with @VisibleForTesting, matching the annotation already used in this class. The same unguarded toString() on this field also appears in ArchivedTimelineV2#readCommit and MetadataConversionUtils, where the right behaviour for a null value is less obvious. Left alone here and called out in the PR instead. Closes apache#17095 * fix(timeline): use StringUtils.objToString and tidy the regression test Review feedback. - readCompletionTime now uses StringUtils.objToString, the existing null-safe toString that HoodieAvroUtils.getNullableValAsString is built on, instead of a local variable and a ternary. - Test: added the missing class javadoc, renamed to match the convention in this area (testReadCompletionTime / testReadCompletionTimeWithoutCompletionTime), dropped the instantTime and action fields that readCompletionTime never reads and which implied a coupling that is not there, and made the assertion messages consistent across both cases. On moving the test onto the real archiving harness in hudi-client-common: tried it, and it does not reproduce this bug. Details in the review thread. * test(timeline): cover the null completion time on the real archived read path Adds testReadCompletionTimeWithoutCompletionTime to TestCompletionTimeQueryView. It archives an instant carrying no completion time through LSMTimelineWriter and reads it back through the archived timeline, so the fallback in readCompletionTime is exercised on the path that actually broke. Reverting the fix makes it fail with the HUDI-9655 NPE. The test asserts LSMTimelineWriter's exception handler collected nothing. That handler is optional and the write loop swallows per-instant failures, so without the assertion a failed archive write would leave the test passing against an empty archive. With real-path coverage the mocked TestCompletionTimeQueryViewV2 is redundant, so it goes, and readCompletionTime returns to private. * fix(timeline): null-safe the other two reads of the archived completionTime Review question: the same raw record.get(COMPLETION_TIME_ARCHIVED_META_FIELD) .toString() also lives in ArchivedTimelineV2#readCommit and MetadataConversionUtils#createMetaWrapper. Checked, and both do NPE on the same record shape - createMetaWrapper demonstrably, at line 174, on a record with the field left unset. Both read the same LSM records as the query view, so the trigger is identical: a table archived before completionTime existed. Add ArchivedTimelineV2#completionTimeOrInstantTime so the two sites cannot drift, and route both through it. Falling back to the instant time is the behaviour CompletionTimeQueryViewV2#setCompletionTime already documents for these records, so this follows existing precedent rather than inventing a rule. Both sites build a COMPLETED HoodieInstant, and leaving the completion time null there would only move the failure to whatever compares it. CompletionTimeQueryViewV2#readCompletionTime is left as is: it hands a possibly-null value to setCompletionTime, which owns the fallback, so it needs nothing further. Note the two sites already null-check the neighbouring nullable fields, metadata and plan, so completionTime was the odd one out rather than a deliberate choice. --------- Co-authored-by: voon <voonhousu@gmail.com> (cherry picked from commit 55c7a30)
apache#19483) * perf(trino): cache decimal Avro schema in HudiAvroSerializer instead of parsing per value AvroDecimalConverter built and JSON-parsed an Avro schema for every decimal cell on the record read path. Cache the schemas by (precision, scale) and build them with LogicalTypes on miss. Also cache buildRecordInPage field positions per record schema instead of doing a name lookup per record per column, and make writeRow's anonymous-field name fallback lazy. Fixes apache#19361 * address review: replace bit-packed decimal cache key with precision * 100 + scale (cherry picked from commit 2f8a725)
…tive timeline midpoint (apache#19239) * fix(meta-sync): advance last commit time synced when it trails the active timeline midpoint * docs(meta-sync): clarify the marker advances on catalog-visible change, not on data write * docs(meta-sync): say "last commit time synced" instead of "marker" in the javadoc * test(meta-sync): unit-test the timeline-midpoint helper and take the midpoint over completed commits Compute the midpoint over completed commit instants only so an inflight instant cannot shift it, and add TestHiveSyncToolTimelineMidpoint covering the present/empty guards and the inflight-vs-completed boundary. * test(meta-sync): narrow the timeline midpoint to completed commits Use getCommitsTimeline().filterCompletedInstants() so clean, rollback, and other non-commit instants cannot shift the midpoint, and add a test that pins the narrowing to the commits timeline. * refactor(meta-sync): read completed commits via metaClient.getCommitsTimeline() Keep the helper and its test identical across lines by reading the commits timeline from metaClient.getCommitsTimeline(), which is available everywhere, rather than the HoodieTimeline interface method. * test(meta-sync): rename midpoint test for clarity Rename midpointIsTakenOverCompletedCommitsOnly to midpointIsComputedFromCompletedCommitsOnly. (cherry picked from commit 99ceae1)
…e#19470) * fix(fs): stop depending on the optional FileSystem#getScheme() FileSystem#getScheme() is optional in Hadoop: the base implementation throws UnsupportedOperationException, and proxy implementations such as Presto's PrestoS3FileSystem do not override it. Hudi called it unguarded on filesystems it did not implement, so opening a log file on such a filesystem failed with "Not implemented by the PrestoS3FileSystem FileSystem implementation" instead of reading anything (HUDI-4602). Adds HadoopFSUtils#getScheme(FileSystem), which returns fs.getScheme() and falls back to fs.getUri().getScheme() when it is unimplemented. getUri() is abstract, so every implementation supplies it, and its scheme is what getScheme() returns wherever both are present. This is the same conclusion as apache#793, which stopped HoodieWrapperFileSystem calling getScheme() on the filesystem it wraps. Routes the seven unguarded call sites through it: isGCSFileSystem and isCHDFileSystem (the reported read path), registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath, HoodieRetryWrapperFileSystem#getScheme, WriteMarkersFactory's HDFS gate, and HoodieHadoopStorage#getScheme, which is what the seven HoodieStorage#getScheme callers reach. isGCSFileSystem's comparison is also flipped to put the constant first, matching isCHDFileSystem, so a filesystem whose URI carries no scheme returns false rather than throwing NullPointerException. * test(fs): say which branch of the helper each assertion covers Review nit: the assertion messages did not make clear what had gone wrong. Each now names the filesystem and the branch of the helper it pins - LocalFileSystem overriding getScheme() so the helper returns what it reports, FilterFileSystem not overriding it so the helper falls back to getUri().getScheme(). * fix(fs): fail loudly on an unresolvable scheme, and cover the sites this reroutes Review feedback, all of it well founded. The fallback no longer returns null. InLineFileSystem is the counter-example in this module: getScheme() is "inlinefs" while getUri() is URI.create("inlinefs"), which has no colon and so no scheme, so the two are not interchangeable and the javadoc claim that they agree was simply wrong. A null surfaced far from the cause as "does not support scheme null" or "Unsupported scheme :null" with the UnsupportedOperationException discarded; it now throws with that exception chained. HoodieException rather than HoodieIOException, since the latter only accepts an IOException cause. HoodieHadoopStorage memoizes the scheme. On a filesystem without getScheme() the fallback costs a thrown-and-caught exception, and this is called once per log block via StorageSchemes.isWriteTransactional and three times per immutable-file write via needCreateTempFile. A lazy field keeps all five constructors untouched. Test coverage for what this actually reroutes, none of which any test reached: - registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath (the write path) and HoodieHadoopStorage#getScheme, via a LocalFileSystem subclass whose getScheme() throws, registered as fs.file.impl so it is reached through FileSystem.get. - isGCSFileSystem and isCHDFileSystem, which become reachable for proxy filesystems for the first time here and select different stream wrappers: a scheme-less filesystem reporting gs:// now yields SchemeAwareFSDataInputStream and ofs:// yields BoundedFsDataInputStream. - the new unresolvable-scheme failure. TestFSUtilsWithRetryWrapperEnable#testGetSchema has been inert since HUDI-5286 added it: it asserted on HoodieWrapperFileSystem#getScheme, which is uri.getScheme() and never dispatches into the retry wrapper, and FakeRemoteFileSystem overrode getScheme() to delegate to a real LocalFileSystem so it could not throw. Dropping that override gives the fake the PrestoS3FileSystem shape and the assertion now targets the retry wrapper, so it guards both HUDI-5286 and this change. Verified: it fails with the pre-PR helper. Also drops the try/catch in convertToHoodiePath that only rethrew HoodieIOException unchanged, dead since ef70de2, and the duplicated fixture and redundant nested close in TestHadoopFSUtils. (cherry picked from commit 70a5a4d)
… timestamp logical type (apache#19384) Promoting a bare long column to a timestamp logical type during writer-schema deduction is now uniformly gated behind the per-field override hoodie.write.timestamp.logical.type.overrides: rejected with an actionable error when the field has no override, and applied when it does. This holds for all four target types (timestamp-micros, timestamp-millis, local-timestamp-micros, local-timestamp-millis) and in both reconcile paths (reconcileSchema and reconcileTimestampLogicalType). Previously, long to local-timestamp was already override-gated, but long to UTC-timestamp was silently allowed on the default (non-reconcile) write path, because isGatedTimestampChange did not treat it as a gated change. A bare long carries no precision signal (millis vs micros), so silently attaching a UTC timestamp logical type could mislabel stored values. This makes the two cases behave identically. Timestamp precision flips are unchanged. timestampPrecisionChangeError is made public so tests assert the exact message without duplicating its format. Tests: TestSchemaChangeUtils and TestAvroSchemaEvolutionUtils cover the gating on both reconcile paths, with and without an override, for all four targets including nested fields; TestHoodieDeltaStreamer.testLongToTimestampPromotionGated exercises the promotion end to end. (cherry picked from commit e4718be)
…split_size (apache#19478) * fix(trino): report real block size in HudiTrinoStorage and slice splits by target_split_size HudiTrinoStorage hardcoded blockSize=0 in convertToPathInfo and getPathInfo, the hudi-trino counterpart of trinodb/trino#29842. Report the file length as the block size instead, matching the upstream fix at the storage layer. HudiSplitFactory used max(target_split_size, blockSize) to size base file splits. With the storage layer now reporting length as block size, that max() would silently disable the target_split_size knob, so make the policy explicit: slicing is governed solely by target_split_size. Also fail fast on a non-positive target, which previously looped forever. Covers apache#19231. * fix(trino): validate target_split_size at config time and pin the split sizing tests Addresses review feedback on apache#19478: - Move the non-positive target split size guard from the middle of createSplitsForBaseFile into the HudiSplitFactory constructor, so it covers every split path instead of only base file slicing past the fileSize == 0 early return. createHudiSplits becomes private, leaving the constructor as the single entry point that can be handed a bad target. - Reject a zero target at config time as well: @MinDataSize("1B") on HudiConfig.getTargetSplitSize and validateMinDataSize on the target_split_size session property, matching the shape already used by parquet_small_file_threshold. The constructor check stays as a backstop. - Set the TestHudiSplitFactory fixture block size to the base file length, which is what HudiTrinoStorage now reports. The old fixed 8MB fixture was below the 128MB target, so the default-target tests passed on master unchanged and did not pin the max() removal. With the block size tracking the file length, restoring max(target, blockSize) fails 5 tests, including the 500MB default-target case. (cherry picked from commit 66b9c6e)
…es (apache#19495) * perf(trino): drop the decimal schema cache and memoize prefilled column values Follow-up to apache#19483, addressing wombatu-kun's review comments. The decimal schema cache was unnecessary rather than merely mis-keyed. Avro's DecimalConversion.fromBytes reads only the scale (it never touches precision, and ignores its schema argument), and Decimals.encodeShortScaledValue then calls setScale to that same scale, which returns the BigDecimal unchanged. The pair reduces to new BigInteger(fixed.bytes()).longValueExact(), so AvroDecimalConverter and its ConcurrentHashMap are deleted instead of re-keyed. PrefilledColumnValues resolved every value through HiveUtil.getPrefilledColumnValue on each call, and appendTo runs once per prefilled column per record. Every input is a split constant, so the resolved value is now memoized per column. Tests: the decimal test now drives the public appendTo path over scales, signs and the widest short decimal rather than the deleted converter; it passes against both the old and new implementations. Added repeated resolution of a hive-null column, the case a naive memo would get wrong. * refactor(trino): rename the uncached prefilled resolver to computeNativeValue * perf(trino): collapse the prefilled memo hit path to a single hash lookup Addresses wombatu-kun's review comment on apache#19495. containsKey-then-get was two hash lookups per prefilled column per record on the buildRecordInPage path. An UNRESOLVED sentinel with getOrDefault does it in one, while still distinguishing a not-yet-resolved column from one resolved to null (the hive-"\N" convention, and the lenient fallback for a column the split cannot provide). The map still stores real nulls, so it stays a HashMap. * test(trino): build the decimal fixed the way Avro writes it Addresses wombatu-kun's two review comments on apache#19495. The short-decimal cases built the Fixed from BigDecimal.unscaledValue().toByteArray(), the minimal two's-complement encoding, and sized the schema to those bytes. Avro's DecimalConversion.toFixed instead left-pads to the schema's fixed size with the sign byte, so a real decimal(10,2) is always five bytes. The negative cases were one and two bytes wide, meaning no case exercised sign extension across padding -- the thing the decode is most likely to get wrong. The fixture now sizes the schema from the precision and runs the value through Avro's own conversion, matching how TestHudiUtilColumnHandles builds its decimal fixed schema. Cases are unchanged and still pass; -0.07 now decodes from FF FF FF FF F9 rather than F9. Also drops an overreaching claim on the hive-null repeats: a null-check memo returns null on every call too, so the repeats do not rule it out. They cover what they actually cover, that both read paths keep returning null once the memo is populated. (cherry picked from commit c2e884a)
CDC iterators leaked ExternalSpillableMap instances when construction or iteration failed: the image manager was never closed on the failure path, leaving spill files behind. Route cleanup through a shared CloseableUtils.closeSuppressing helper, close the image manager via try-with-resources, and retain CDC images across child splits. (cherry picked from commit f41e8e3) Cherry-pick adaptations: - HoodieSplitReaderFunction: the private closeSuppressing helper being deleted is typed HoodieFileGroupReader<RowData> here rather than HoodieRecordReader<RowData>, since the LSM reader refactor (apache#18987, apache#19079, apache#19307) is not on this branch. Same deletion, different pre-existing signature. The shared replacement takes AutoCloseable and HoodieFileGroupReader implements Closeable, so call sites are unchanged. - CdcIterators: upstream drops the FormatUtils import and keeps HoodieRowDataFileReader / InternalSchemaManager as context. This branch's copy of the file never imported the latter two and does not reference them, so only the FormatUtils import is dropped. Adding the other two would be unused imports and fail checkstyle. The static import of FormatUtils.buildAvroRecordBySchema is unaffected. - Dropped the TestCdcImageManager and TestCdcIterators changes. Both test classes arrive with apache#19402, which is not on this branch, and both assert behavior from that commit's production half: TestCdcImageManager expects skipBytesToRead to throw EOFException, which without apache#19402 loops forever rather than failing. Importing them would add a hanging test. The fix therefore lands without its CDC test coverage. TestCloseableUtils is included, so the shared helper itself is covered.
…opStorage The cherry-pick of apache#19470 (b8a09bf) brought master's org.apache.hudi.common.util.Lazy import, but the hudi-common core package reorganization (apache#19195) that moved Lazy there is not on this branch. Lazy is still org.apache.hudi.util.Lazy here, so hudi-hadoop-common failed to compile with "cannot find symbol: class Lazy", breaking every downstream module. Note: 14 files under hudi-trino carry the same master-only Lazy import. That module is profile-gated off by default (-Phudi-trino) so it does not break the default build, and is left alone here.
…e metric name (apache#19476) * fix(metrics): do not drop the whole CloudWatch batch on one unmappable metric name stageMetricDatum derives the CloudWatch Table dimension from the part of the metric name before the first dot, so a name without one cannot be mapped. It threw for that case, and report() stages every metric into one list before calling putMetricData, so throwing part-way through meant the request was never sent: one unmappable name cost every metric in the interval. ScheduledReporter then suppresses the exception, so the user saw a log line and an empty dashboard. Such names still reach the reporter on master. HoodieMetadataMetrics#setMetric registers gauges with no prefix, unlike Metrics#registerGauges, so getStats contributes a bare partitionCount and BaseTableMetadata a bare lookup_meta_index_bloom_filters_file_count. Skip the metric that cannot be mapped and publish the rest, logging the name once rather than every interval. The intent of the previous check is kept - the metric is still not reported under a wrong table, and is now named in a warning - without taking the other metrics down with it. Fail-fast was never reachable here anyway, since ScheduledReporter suppresses whatever report() throws. * fix(metrics): also skip metrics whose table name is empty, and cover the log-once path Review feedback. The headline example in this PR was unreachable and I have replaced it. partitionCount comes from HoodieMetadataMetrics.getStats only when detailed == true, while the gauge-registering path calls getStats(false, ...); the only detailed=true caller is HoodieBackedTableMetadata.stats(), whose sole consumer prints the map in hudi-cli and registers nothing. The fixture and javadoc now use lookup_meta_index_bloom_filters_file_count, which BaseTableMetadata registers on the normal bloom-index read path. An empty first segment was still losing the batch, which is the same bug class this PR exists to fix. hoodie.metrics.reporter.metricsname.prefix defaults to "" and Metrics#registerGauges still joins it with a dot, so ".foo" splits into two parts, passed the length check, and asked CloudWatch for an empty Table dimension value - which it rejects for the entire PutMetricData request. The guard now also rejects an empty first segment. The warning no longer claims a <table>.<metric> convention that Hudi does not follow: no metadata metric carries a table name, and an operator has no knob that changes the names being skipped. It now names the prefix config and points at apache#19507 for the producer side. Three tests added: the empty-table-name case, an interval where every metric is unmappable asserting that no empty PutMetricData request is sent, and one that reports twice and asserts a single WARN, so the once-per-name set is no longer uncovered. Both new guards fail the suite when reverted. (cherry picked from commit c63c9bf)
…connector (apache#19217) * test(integ-test): add Trino E2E harness for the RFC-105 connector Revives the testcontainers Trino harness from the stale branch, adapted: - TrinoService + ITTestTrino{Smoke,StockTicks,CustomType}, gated on a new docker-compose 'trino' profile (-Dcompose.profiles=trino) so existing hive-sync CI rows are unaffected - docker/trino: Dockerfile (FROM trinodb/trino:481, bakes the plugin dir built from the Trino repo's thin trino-hudi shim + etc configs), overlay-aware entrypoint (jar-bearing TRINO_PLUGIN_DIR mount overrides the baked plugin for fast iteration), build_image.sh - trinocoordinator service (profile-gated) in the spark402 compose pair * ci(trino): complete the Trino E2E pipeline for the RFC-105 connector The harness from the previous commit could not actually run: nothing in the repo could assemble the Trino plugin directory (the RFC-105 shim is not yet released upstream), the stock-ticks seed fixture was never committed, and no workflow activated the trino compose profile. - docker/trino/shim: standalone maven project mirroring the upstream trinodb/trino plugin/trino-hudi shim (parent io.trino:trino-root:481 from Maven Central, packaging trino-plugin, depends on org.apache.hudi:hudi-trino). Carries its own thin HudiPlugin class because trino-maven-plugin's service descriptor generator only scans the module's own classes; never deployed (maven.deploy.skip). Build with package, never install. - docker/trino/Dockerfile + overlay-entrypoint.sh: preserve the stock image's plugin/hudi/hdfs loader jars at /opt/hudi-hdfs-lib and re-attach them when the baked or overlaid plugin dir lacks them (shim-assembled dirs always do; io.trino:trino-hdfs:zip is not on Maven Central, it only ships in the server tarball / base image). - docker/demo/sparksql-stock-ticks-trino.commands: the COW + MOR seed fixture ITTestTrinoStockTicks references, jdbc-mode hive sync like the existing custom-type fixtures. - .github/workflows/hudi_trino_e2e.yml: JDK 17 reactor build, JDK 25 connector + shim build, local image build via build_image.sh, then ITTestTrino* against the spark402 compose stack with -Dcompose.profiles=trino. No trinodb/trino checkout and no DockerHub publish anywhere in the flow. - docker/trino/.gitignore: anchor plugin/ to /plugin/ so it does not swallow the shim's io/trino/plugin source package. - READMEs: local E2E flow + overlay fast loop; drop stale hudi-trino-plugin path comments. * ci(trino): TEMP: push-trigger the E2E workflow on the dev branch workflow_dispatch cannot target a workflow that does not exist on the default branch yet. Drop this commit before merging upstream. * fix(trino): drop jts-core provided override in the E2E shim pom Trino 481's SpiDependencyChecker rejects provided scope for org.locationtech.jts:jts-core: it is not part of the 481 SPI surface (trino-spi 481 depends only on jackson-annotations, slice, and the opentelemetry api/api-incubator/common/context set). jts-core now flows into the assembled plugin dir at its transitive scope. The upstream 482-SNAPSHOT shim declares it provided because the SPI gains jts there. * test(integ-test): surface Trino boot failures in the E2E pipeline All three ITTestTrino* classes failed with 'execInContainer can only be used while the Container is running': the coordinator dies at startup and nothing captured why (compose teardown discards the container). - ITTestBaseTestcontainers: stream trinocoordinator logs into the test output via Slf4jLogConsumer when the trino profile is active, so a startup crash leaves its root cause in the failsafe report. - TrinoService.waitUntilReady: fail fast with a pointed message when the container is no longer running instead of exhausting 18 retries against a dead container (saves ~3 min per class). - hudi_trino_e2e.yml: assert the assembled plugin dir is populated and carries the services jar; add a standalone smoke-boot of the built image (docker run + SELECT 1 via the CLI, boot log dumped on failure) so image-level boot failures surface ~30 min before the IT step. * fix(trino): drop GCLocker JVM flags the Trino 481 JDK no longer accepts The standalone smoke-boot caught the coordinator crash root cause: 'Unrecognized VM option GCLockerRetryAllocationCount=32'. The flag was carried over from Hudi's JDK 11/17 CI OOM workaround, but the GCLocker was removed in modern JDKs, so the JDK 25 JVM inside trinodb/trino:481 refuses to start and the container dies before any Trino code runs -- which is exactly the 'container is not running' failure all three ITTestTrino* classes hit. * test(integ-test): surface server-side stacks for failing Trino queries The E2E run reached real query execution (ITTestTrinoSmoke fully green, the stock-ticks fixture seeds and syncs correctly) but every table read fails with 'Failed to generate splits for default.<table>' and only the one-line message survives into the report. - TrinoService: run the CLI with --debug so query failures carry the full server-side stack into the assertion message. - hudi_trino_e2e.yml: on failure, print the failsafe *-output.txt tails (where surefire redirects the streamed trinocoordinator logs). * ci(trino): add the ASF license header to the empty-overlay .gitkeep The release licensing check (scripts/release/validate_source_copyright.sh) requires every non-excluded file to contain the ASF header, and an empty .gitkeep cannot. The file only keeps docker/trino/empty-overlay in git as the compose default plugin-overlay mount source; its content is ignored at runtime (the overlay entrypoint only applies an overlay that contains jars), so the header plus a purpose note satisfies the check without touching the shared release script. * test(integ-test): address Trino E2E review on teardown, naming and MOR signal - Guard @afterall teardown in ITTestTrinoStockTicks/ITTestTrinoCustomType: JUnit runs @afterall even when the @BeforeAll assumption aborts setupOnce() before initializeServices(), so a non-trino stack NPE'd on the null spark service instead of skipping cleanly. - Rename the BothPartitionsVisible tests: the emptied dt=2024-01-02 partition is invisible by design, so the old names claimed the opposite of the check; pin the invisibility with an absence assertion. - Give stock_ticks_mor a log-only UPDATE delta so the _ro and _rt views diverge: _ro must keep serving the base row (now absence-pinned) and two new _rt tests verify the merged read, so the MOR rows exercise a genuinely MOR read path instead of the base-file-only path COW already covers. * ci(trino): harden the Trino E2E workflow and shim per review - Drop the TEMP dev-branch push trigger: only squash merge is enabled in .asf.yaml, so there is no separate commit to drop and the line would ship to master with the branch head. - Widen the demo paths filter to docker/demo/**: the ITs drive several demo fixtures beyond the stock-ticks commands file, and edits to those must re-trigger this pipeline. - Carry bot.yml's maven.wagon retry flags in MVN_ARGS: the JDK 17 step is the cold-cache full-reactor build those flags were added for. - Redirect failsafe test output to files so the on-failure dump step has the *-output.txt files it tails; failsafe defaults the redirect off, so the loop never matched anything before. - Derive dep.hudi.version from the reactor pom when building the shim: cut_release_branch.sh's versions:set cannot bump the out-of-reactor shim pom, and a stale literal could resolve silently from the actions maven cache instead of failing loudly. - Hard-disable install for the shim pom (maven.install.skip) so a stray install cannot publish a stub at the real io.trino:trino-hudi coordinates, and dockerignore the shim tree so shim/target stays out of the docker build context. * test(integ-test): retire the legacy trino-coordinator demo path The trino-coordinator-1 service is gone from every compose file, the ITTestHoodieDemo trino steps have been commented out since the HUDI-8269/HUDI-8270 breakage, and the integ2 Trino E2E suite now covers the same stock-ticks queries against the native RFC-105 connector. Remove the dead ITTestBase helpers and the ITTestHoodieDemo constants, copy steps and private test methods; the three trino-*.commands demo files they consumed were dropped in the preceding fixture commit. * ci(trino): add the ASF license header to the trino .dockerignore The release licensing check (scripts/release/validate_source_copyright.sh) greps every file in the source release tree for the ASF header. The release rsync (create_source_directory.sh) strips .gitignore files but has no exclude for .dockerignore, so the file ships in the source release and must carry the header, same as the empty-overlay .gitkeep. Docker treats leading # lines as comments, so the ignore behavior is unchanged. * ci(trino): use bot.yml's aether resolver retry flags and pin the GOOG row count - MVN_ARGS carried maven.wagon.* names, which Maven 3.9's native resolver transport ignores; swap in bot.yml's five aether.connector.http.* flags verbatim so the 429/5xx retry actually applies to the cold-cache build. - The _ro/_rt projected-column asserts matched one row substring, which still passes when an unmerged base row is returned next to it; pin the symbol count to one. * test(integ-test): correct the Trino custom-type javadoc and drop the dead Spark 3.5 guards Three doc/scope corrections from review on ITTestTrinoCustomType: - the class javadoc credited ITTestCustomTypeHiveSync with a re-seeding @BeforeAll and a cleaning @afterall, but it seeds inside each @test and cleans in @AfterEach. What actually keeps the two classes from contaminating each other is tearDownDockerCompose dropping the whole stack between classes; say that instead. - trinocoordinator exists only in the spark402 compose pair, so this class either skips entirely (no trino profile) or runs on a Spark 4.x stack -- isSpark4Compose() is always true when it runs. Drop the conditional VARIANT seed and the seven assumeSpark4Compose() guards, and replace the "on Spark 3.5 the BLOB and VECTOR coverage still runs" claim with the reason VARIANT is unconditional here. - the two EmptiedPartitionInvisible tests claimed to exercise partition pruning, but GROUP BY dt only emits groups for partitions that still hold rows, so it cannot distinguish a connector that prunes from one that lists -- and dt=2024-01-02 stays registered in the metastore either way. Reword both comments to state what is actually asserted. Also trims the now-stale usage guidance on isSpark4Compose(), which no longer has a @BeforeAll-seeding caller. * ci(trino): widen the E2E path filter to hadoop.env and fix the shim/catalog docs Review round 3 on the Trino E2E pipeline: - ITTestBaseTestcontainers copies docker/compose/hadoop.env into the generated compose dir and all nine services in the spark402 pair load it, but the workflow path filter matched only the spark402 YAMLs. Add hadoop.env to both the push and pull_request lists. - hudi.properties dated the native connector at Trino 472. plugin/trino-hudi first appears upstream at tag 398 (404 at 397), so correct the number. - The shim build steps in hudi-trino/README.md and docker/README.md used the pom's literal dep.hudi.version, which the workflow itself documents as going stale because cut_release_branch.sh cannot bump a pom outside the reactor. Derive it from help:evaluate the same way the workflow does. - Rename Containers.TRINOCOORDINATOR to TRINO_COORDINATOR for consistency with SPARK_MASTER / ADHOC_1. The value stays "trinocoordinator" to match the compose service name. * ci(trino): trigger the E2E pipeline on root pom.xml and all of hudi-integ-test Two gaps in the path filter: - root pom.xml owns trino.version (481), which the shim pom's parent coordinate, the Dockerfile TRINO_VERSION arg, build_image.sh's default and four hardcoded trino-hudi-481 paths in this workflow all track by hand. A bump there needs this pipeline to run; hudi_trino_ci.yml already lists pom.xml for the same reason. - the test entry was scoped to integ2/**, but the IT step runs `mvn verify -pl hudi-integ-test`, which compiles the whole module. This PR's own edits to integ/ITTestBase and integ/ITTestHoodieDemo sit outside integ2/, so a break there would fail this pipeline without triggering it. Widen to hudi-integ-test/**. Also folds the per-entry comments into one rationale block above the push list, with the pull_request list pointing at it, so the two stay easy to diff against each other. (cherry picked from commit 0db5246)
…9503) HoodieLookupTableReader was left open when a cache reload attempt failed, leaking the underlying input format on every failed reload. Close the reader after each attempt, successful or not, and route the failure path through CloseableUtils.closeSuppressing so a close error does not mask the original exception. (cherry picked from commit a44572b) Cherry-pick adaptation: - TestHoodieLookupFunction: the conflict rendered master's whole region, which includes testRocksDBCacheLifecycleAndLookupFailure. That test is not on this branch and apache#19503 does not add it, so only the commit's own changes were applied: the new testReaderIsClosedWhenCacheReloadFails, the FailingLookupTableReader helper, and the newLookupFunction parameter retyped from CountingLookupTableReader to HoodieLookupTableReader. - Added the assertThrows static import, which the new test needs and this branch's copy of the file did not already have. Both production files and the new TestHoodieLookupTableReader apply byte-identically to the upstream delta.
…ow write support (apache#19512) * fix(spark): preserve the Avro fixed-size decimal width in the Spark row write support The Spark row writer sized decimal FIXED_LEN_BYTE_ARRAY columns from Decimal.minBytesForPrecision(), discarding an Avro fixed(N) size wider than the precision-minimal width and diverging from the Avro write path. Honor the declared fixed size from the resolved HoodieSchema at both the schema converter and the value writer, resolving the padding buffer once per column. * Guard against an Avro fixed size smaller than the precision-minimal decimal width Fail fast with a diagnosable message if a decimal fixed(N) declares fewer bytes than minBytesForPrecision, instead of an opaque negative-length Arrays.fill later. * Unit-test decimalFixedLen directly for hudi-spark-client coverage Make decimalFixedLen package-private and cover its branches (min-width fallback and honored Avro fixed size) from hudi-spark-client's own test module, where the full write support cannot be constructed. * Extract decimal sign-extension padding into a testable static helper Move the fixed-length padding out of the makeWriter lambda into padDecimalToFixedLength so it can be unit-tested directly from hudi-spark-client (covering the exact-width, positive-pad, and negative sign-extension cases), where the full write support cannot be constructed. * Trim redundant Javadocs on the decimal helpers Drop the padDecimalToFixedLength Javadoc and reduce decimalFixedLen's to a single line. * Rename decimalFixedLen to resolveDecimalByteLength and use assertFalse for the clustering check Address review nits: resolveDecimalByteLength reads more clearly at the call sites than decimalFixedLen, and assertFalse(isEmpty()) is more natural than assertTrue(!isEmpty()). (cherry picked from commit 9a30ecd)
…y produces (apache#19477) * fix(metrics): tell a CloudWatch reporter version mismatch apart from a missing bundle ReflectionUtils.loadClass collapses NoSuchMethodException, InvocationTargetException, InstantiationException and IllegalAccessException into one HoodieException with a fixed message, so a hudi-aws jar built against a different Hudi version surfaced only as "Unable to instantiate class org.apache.hudi.metrics.cloudwatch.CloudWatchMetricsReporter". Working out that the class had resolved and only its constructor had not matched meant digging the buried NoSuchMethodException out of the stack. Translate that cause too: report that the class was found but has no (HoodieMetricsConfig, MetricRegistry) constructor, and that the remedy is a hudi-aws-bundle matching the Hudi bundle in use. That is deliberately different from the missing-module message, which says to add the bundle - absent and mismatched need different fixes, and conflating them is what made the message unhelpful. metricsReporterFactoryLeavesNonClassNotFoundFailuresUntouched used a NoSuchMethodException to stand for "some other failure", which is now handled, so it switches to an InvocationTargetException and is renamed accordingly. * test(metrics): fix a dropped word in an assertion message Review nit: "A failure that is neither cause must not be rewritten" was missing the two cases it refers to. * fix(metrics): route the reflection failures a CloudWatch skew actually produces Review found that the constructor-mismatch branch cannot fire for the case its message described, and I reproduced it: with a released hudi-aws-bundle-1.1.0-rc1 on the classpath against this branch's hudi-common, STEP1 Class.forName OK -> class org.apache.hudi.aws.metrics.cloudwatch.CloudWatchMetricsReporter STEP2 getConstructor THREW java.lang.NoClassDefFoundError: org/apache/hudi/config/metrics/HoodieMetricsConfig is NoSuchMethodException? false is Error (escapes catch(Exception))? true Class#getConstructor resolves the parameter types of every public constructor, not just the requested one, and apache#19193 moved HoodieMetricsConfig out of org.apache.hudi.config.metrics with no stub there. So a clean older bundle dies on the vanished type, as an Error that ReflectionUtils never wraps, and the previous catch (HoodieException) never saw it. NoSuchMethodException only arrives when the classpath carries a stale or duplicate copy - which is also what apache#12902 actually was: that report had matching 0.15.0 artifacts, and 0.15.0 did declare the requested constructor. Three causes, three remedies: - ClassNotFoundException -> hudi-aws absent, add the bundle. - NoClassDefFoundError -> built against a different Hudi; the message quotes the type that vanished, which is the strongest evidence of skew available. - NoSuchMethodException -> stale or duplicate copy; look for more than one Hudi version, a leftover hudi-common or hudi-client-common being the usual culprit. Parameter types are printed fully qualified. The released bundle declares (org.apache.hudi.config.metrics.HoodieMetricsConfig, org.apache.hudi.com.codahale.metrics.MetricRegistry) while this Hudi requests (org.apache.hudi.common.config.metrics.HoodieMetricsConfig, com.codahale.metrics.MetricRegistry) - under simple names both read as (HoodieMetricsConfig, MetricRegistry) and the error looks like it is lying. Tests: both ClassNotFound tests now assert their own distinguishing phrase, since every string they checked also appears in the mismatch message; the mismatch test pins the NoSuchMethodException into the cause chain, which nothing did before; and one test drives the real ReflectionUtils with a fixture whose only public constructor does not match, so the suite no longer asserts only the shapes the production code assumes. That gap is how the NoClassDefFoundError case stayed invisible. (cherry picked from commit c4b3893)
…ckage The cherry-pick of f270000 (apache#19384) carried master's org.apache.hudi.common.schema.internal package for Type, Types, AvroSchemaEvolutionUtils and SchemaChangeUtils. That package name arrived with apache#19195's hudi-common reorganization, which is not on release-1.2.1, so the four imports resolved to nothing and hudi-utilities failed to test-compile. On this branch the classes still live under org.apache.hudi.internal.schema, with identical signatures for the members the test uses, so this is a package rename only. The imports also move down the block, since "internal" sorts after "hive" where "common.schema.internal" sorted after "common.schema". Only the test imports were affected. The same pick's hudi-common changes had already been adapted to the release paths.
integration-tests-hive-sync declared "needs: changes", but no job with that id exists on this branch. GitHub rejects the workflow at parse time, so every push to release-1.2.1 produced a Java CI run that failed in 0s with zero jobs and no matrix ran at all. PR apache#19544 saw no Java CI check for the same reason. The "changes" job is a path-relevance gate that master added in b8cab71 (apache#18598). The release line does not have it: release-1.2.0 has 27 jobs and zero needs.changes references, and gates its coverage steps on a bare "if: always()". The 11 references here came in as cherry-pick residue, the hive-sync ones with adebc4c (apache#19203), and should have been stripped when those picks were adapted. Strip them rather than backport the gate, which matches the release line and keeps the full matrix running unconditionally. This also fixes a quieter bug: test-flink-1 and test-flink-2 referenced needs.changes.outputs.relevant without declaring the dependency, so those two coverage and Codecov steps evaluated false and never ran.
…pache#19464) * fix(client): report the completed timeline action in the clustering write commit callback Signed-off-by: codope <sagarsumit09@gmail.com> * test(client): pin completed clustering action, share callback Signed-off-by: codope <sagarsumit09@gmail.com> * fix(client): use REPLACE_COMMIT_ACTION constant in clustering callback Signed-off-by: codope <sagarsumit09@gmail.com> --------- Signed-off-by: codope <sagarsumit09@gmail.com> (cherry picked from commit 79fedcf)
…supplier overload checkState(boolean, Supplier<String>) threw IllegalArgumentException, a copy-paste slip from the checkArgument overload directly above it. The other two checkState overloads throw IllegalStateException, as does the javadoc contract, so which exception a caller saw depended only on whether it passed a String or a lambda. Surfaced by TestHoodieBackedTableMetadataDataCleanup, which the cherry-pick of 98462b4 (apache#19359) brought over: it asserts IllegalStateException from HoodieBackedTableMetadata.readSecondaryIndexLocationsWithKeysV1, and that check uses the supplier form. Master fixed this in 2bed01a (apache#19212), a *Utils consolidation refactor that is not otherwise being backported, so take the one-line fix on its own. Only three call sites use the supplier form -- ClusteringExecutionStrategy:137 and HoodieBackedTableMetadata:244,524 -- and all three are genuine state checks. testSecondaryIndexUnsupportedVersion still passes: the IllegalArgumentException it asserts comes from a literal throw in the version dispatch, not from checkState.
hudi-trino/pom.xml declared parent org.apache.hudi:hudi:pom:1.3.0-SNAPSHOT while this branch is 1.2.0, so Maven could not read the project at all: Non-resolvable parent POM ... Could not find artifact org.apache.hudi:hudi:pom:1.3.0-SNAPSHOT and 'parent.relativePath' points at wrong local POM That failed test-hudi-trino-plugin before it compiled a line. Neither this pom nor the E2E shim exists on release-1.2.0; both arrived via cherry-picks carrying master's version, and a parent version cannot be a property, so it has to be the literal branch version. docker/trino/shim/pom.xml's dep.hudi.version default is fixed for the same reason. It is latent rather than fatal, because the E2E workflow derives the value from the reactor pom and overrides it -- the shim sits outside the reactor so cut_release_branch.sh's versions:set cannot reach it.
…ded reads (apache#19520) * perf(flink): use a shared work-stealing split pool for Source V2 bounded reads Bounded reads inherit the streaming split provider, which pins every split to one subtask at discovery and never rebalances, so a subtask that drew a heavier share keeps working while its peers sit idle. On a bounded COW backfill (~16.4K splits, parallelism 32) the readers finished 78 minutes apart and the job took 3.80 h instead of 2.77 h. The affinity pays for itself only in streaming, where a file group accumulates log files across commits and successive splits of one file id must stay on one reader. A bounded read has exactly one split per file group, no cross-commit continuation and no ordering relationship between splits, so any reader can read any split. Add GlobalHoodieSplitProvider, a single shared pool ordered by the existing HoodieSourceSplitComparator whose getNext ignores the subtask id, and select it on the non-streaming branch of HoodieSource.createEnumerator. Streaming keeps DefaultHoodieSplitProvider and the existing assigners. The provider is now chosen on the streaming/bounded branch rather than before it, so restore replays checkpointed splits into whichever provider was chosen. No enumerator change is needed: with one pool, getNext returning empty already means globally drained. Closes apache#19516 * test(flink): cover the metadata-driven bounded incremental branch, not just full scan READ_START_COMMIT=earliest leaves the analyzer's startInstant empty, so IncrementalInputSplits.inputSplits() took its fullTableScan branch and the COW_INCREMENTAL parameter never reached the metadata-driven branch that the one-split-per-file-group argument cites. Split the parameter in two: COW_INCREMENTAL now starts from a real completed commit so fullTableScan is false, and COW_INCREMENTAL_FROM_EARLIEST keeps the full-scan fallback covered. To make the branch observable, the modes that start from a real commit write a last commit that only touches par5 and par6. The metadata-driven branch derives its read partitions from that commit alone, while a full table scan lists par1 through par6, so asserting the split partitions pins which branch produced them. (cherry picked from commit 2490309) Adaptations for release-1.2.1: - HoodieSource keeps its LOG field. Master converted the class to Lombok @slf4j, so the diff's log.info collided with LOG.info. The restructure itself applies unchanged, and the per-file delta is identical to upstream. - Two tests in the TestHoodieStaticSplitEnumerator conflict region, testConstructorWithNullMetricGroup and testHandleSourceEventWithUnknownEventThrows, are master context rather than additions of this commit, so they are not taken. The first would not compile here: it needs a MockSplitEnumeratorContext(boolean) constructor that this branch's mock does not have. - Added a static import for assertFalse. The three new tests use it, but master already imported it for testConstructorWithNullMetricGroup above, so it arrived as context rather than as an added line. This is the only line in which the applied delta differs from upstream.
…eSliceReadFailures The test reflected on HoodieBackedTableMetadataWriter.getLazyMergedFileSlices, which does not exist on this branch. Master added it in 2baa29b (apache#18372), the index-abstraction refactor of the metadata table update path, and that refactor is not backported. The test itself came over with 944e86d (apache#19363), a test-coverage commit, so the caller arrived without the callee. Because the lookup is reflective the compiler could not catch it; it failed at runtime in test-common-and-other-modules: wrapsMetadataReaderAndFileSliceReadFailures:618 » NoSuchMethod HoodieBackedTableMetadataWriter.getLazyMergedFileSlices() Keep the reader half, which exercises mayBeReinitMetadataReader and passes here, and rename the test to match what it now covers. I checked the other five reflective targets in this class -- validateRollback, mayBeReinitMetadataReader, isBootstrapNeeded, metadataTableExists, deletePendingIndexingInstant -- plus all five reflected fields, and only this one is missing. The now-unused Lazy import is removed. The setField helper is kept even though this was its only caller here: master uses it in four other tests, so keeping it avoids a conflict when those are picked later.
…tadata config The hudi-trino module was cherry-picked from master carrying master's package names, so it did not compile here at all. The Trino E2E workflow builds it under JDK 25 and failed on every run with "cannot find symbol" across 21 files; the Trino Connector CI job hit the parent-POM error first and never got far enough to show it. Five packages moved on master in apache#19195's hudi-common reorganization and are not backported. Renamed back to where the classes actually live here: common.util.Lazy -> util.Lazy (14 files) metadata.stats.* -> stats.* (4) core.io.storage.* -> io.storage.* (8) common.util.HoodieStorageUtils -> storage.HoodieStorageUtils common.avro.AvroRecordContext -> avro.AvroRecordContext Also dropped withMetadataIndexPartitionStats(metadataEnabled) from the test initializer. That builder setter does not exist here: partition stats follow the column stats config, and hoodie.metadata.index.partition.stats.enable is marked deprecated with "Partition stats are configured using the column stats config itself". The withMetadataIndexColumnStats call on the preceding line already enables both, so behaviour is unchanged. Master reintroduced the separate toggle. Verified with the CI recipe: JDK 17 install of the hudi deps, then mvn -Phudi-trino -pl hudi-trino compile and mvn -Phudi-trino,hudi-trino-tests -pl hudi-trino test-compile under JDK 25. Checkstyle does not apply to this module (checkstyle.skip is set in its pom).
…che#19522) * fix(flink): preserve Avro fixed decimal widths in Parquet writes Honor declared Avro fixed sizes in both the Flink Parquet schema converter and RowData value writer. Safely sign-extend compact decimals when the declared fixed width exceeds eight bytes. * style(flink): clarify decimal byte length resolver name Rename the helper to describe both fixed-schema resolution and precision-based fallback behavior. Addresses review comment 3718983684. (cherry picked from commit 0425c5c) Adaptations for release-1.2.1: The fix needs each field's HoodieSchema to read the Avro fixed size, and master had already plumbed that through. Here the schema reached RowDataParquetWriteSupport and was converted straight to a RowType and dropped, so the threading is backported alongside the fix, without master's VECTOR work: - HoodieSchemaUtils.getFieldSchema, matching master. - A convertToParquetMessageType(String, HoodieSchema) overload, and a fieldSchema argument threaded through convertToParquetType into ROW, ARRAY and MAP. - ParquetRowDataWriter takes master's (RecordConsumer, boolean, HoodieSchema) constructor. The RowType is derivable from the schema and the GroupType was unused. This also lets the upstream test file apply unchanged. Unlike master, convertToParquetMessageType(String, RowType) does not delegate to the HoodieSchema overload. Converting a RowType to a HoodieSchema is lossy for two types Parquet supports and Avro does not, and both are covered by existing tests here: a map with a non-string key, and a timestamp of precision 9. The field schema is therefore @nullable, and resolveDecimalByteLength already falls back to computeMinBytesForDecimalPrecision when it is absent.
HoodieFlinkMergeOnReadTable.upsert/upsertPrepped on this branch require a
FlinkAppendHandle:
IllegalArgumentException: MOR RowData handle should always be a FlinkAppendHandle
at HoodieFlinkMergeOnReadTable.upsert(HoodieFlinkMergeOnReadTable.java:72)
The test came over with f7eecba (apache#19354) mocking a plain HoodieAppendHandle,
which matches master only because 0311d6c (apache#19067), the native log format
work, relaxed both guards there. That commit is not backported, so the test is
adapted to this branch's contract rather than the guard being widened to match a
refactor we do not have.
The HoodieAppendHandle import stays: the compaction half of the same test still
uses it for a MockedConstruction.
TestHudiUncompactedMetadataTable failed both of its pruning assertions on this branch: testPartitionStatsIndexPruningOverUncompactedStats expected 1 partition but got 2, and testColumnStatsFileSkippingOverUncompactedStats expected fewer than 2 splits but got 2. Neither pruned at all, and the connector logged Column stats index definition is missing or has no source fields defined Partition stats index definition is missing or has no source fields defined canApply hard-required a registered HoodieIndexDefinition to learn which columns the stats indexes cover. On master that definition is written by ColumnStatsIndexer.postInitialization, added by the Indexer abstraction in apache#18348/apache#18372, which is not on this branch. Here only secondary, expression and record indexes get a definition -- getIndexPartitionsToInit covers the first two and createRecordIndexDefinition the third. Nothing registers column_stats or partition_stats, so the connector asked for something this release line never writes and both indexes were dead for every query, not just in these tests. Results stayed correct; every query just fell back to an unpruned scan. Fall back to deriving the columns the way the writer picks them, via HoodieTableMetadataUtil.getColumnsToIndex. This mirrors the Spark reader on this branch, whose ColumnStatsIndexSupport.isIndexAvailable gates only on the metadata partition being present and resolves the indexed columns separately, and it reuses the escape hatch canApply already had one branch up for pre-v8 tables. Treating a column as indexed when it is not is safe. No stats come back for it, and both shouldSkipFileSlice and evaluateStatisticPredicate keep the file when stats are missing, so the cost of guessing wrong is a missed pruning opportunity rather than a wrong result. Partition stats resolve against the column_stats partition, matching the lookup this code already did and the writer, which derives both from existingIndexVersionOrDefault(PARTITION_NAME_COLUMN_STATS, ...). Verified with the CI recipe: JDK 25 compile and test-compile under -Phudi-trino,hudi-trino-tests, then the full module suite. 687 run, 0 failures; the 7 TestHudiUncompactedMetadataTable tests pass and the partition-stats path now logs a real prune down to one partition. The only error is TestHudiMinioConnectorSmokeTest, which needs a Docker daemon this machine is not running.
ITTestTrinoStockTicks.testTrinoReadsMorRtMergedMaxTs and
testTrinoReadsMorRtMergedProjectedColumns failed the query outright:
ClassCastException: class java.lang.String cannot be cast to
class org.apache.avro.util.Utf8
at org.apache.avro.util.Utf8.compareTo
at BufferedRecordMergerFactory.shouldKeepNewerRecord
Base-file records reach the file-group reader through HudiAvroSerializer.serialize,
which builds them from Trino blocks, and Type.getObjectValue hands back a
java.lang.String for VARCHAR. Classic Avro log blocks deserialize inline instead and
yield Avro's own Utf8. The two meet in shouldKeepNewerRecord, which compares the
ordering values directly, and Utf8.compareTo casts its argument to Utf8 -- so any MoR
merged read ordering on a string field threw, whatever the values were.
shouldKeepNewerRecord, BufferedRecords, RecordContext, AvroRecordContext and this
serializer are all identical to master, so this is not a porting slip. Master is green
because its tables are version 10 and default to the native log format (apache#19118): there
the log branch of HudiTrinoReaderContext.getFileRecordIterator runs the SAME serializer,
so both sides are String and agree. HoodieTableVersion on this release line stops at
NINE and current() returns NINE -- version 10 does not exist in 1.2.0 or 1.2.1 -- so
every log block here is a classic Avro block and Utf8 is the only representation both
sides can share.
Emit Utf8 for string-typed record fields in serialize(), looking through nullable
unions. The read-out path is unaffected: writeSlice and the char path already branch on
Utf8 before String.
Covered by TestHudiMorMergeModeSemantics.testEventTimeMergeOnStringOrderingField over a
new fixture with a STRING ordering field. Every existing MoR fixture orders on a long,
where both sides are the same class, which is why only the Trino E2E stock-ticks table
caught this. Verified the test reproduces the exact ClassCastException with the
serializer change reverted and passes with it. Full module suite: 688 run, 0 failures;
the only error is TestHudiMinioConnectorSmokeTest, which needs a Docker daemon this
machine is not running.
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
20 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
Summary and Changelog
Impact
Risk Level
Documentation Update
Contributor's checklist