Skip to content

[AMORO-3853] Support Java 17 and removing reflective/internal dependencies in Flink modules#4124

Open
xxubai wants to merge 14 commits intoapache:masterfrom
xxubai:java17-experimental
Open

[AMORO-3853] Support Java 17 and removing reflective/internal dependencies in Flink modules#4124
xxubai wants to merge 14 commits intoapache:masterfrom
xxubai:java17-experimental

Conversation

@xxubai
Copy link
Copy Markdown
Contributor

@xxubai xxubai commented Mar 17, 2026

Why are the changes needed?

The mixed Flink modules still had several Java 17 blockers, including hard-coded build/toolchain settings, runtime failures caused by JDK proxy and module encapsulation, and brittle reflection against Flink private internals.

This patch makes the mixed Flink Java 17 path buildable, testable, and packageable again.

Close #3853.

Brief change log

  • Align Java 17 build settings across affected modules and remove hard-coded JDK/toolchain constraints.
  • Reduce Flink-side dependence on private/internal reflection by switching to public APIs or explicit wrappers where possible.
  • Upgrade dependencies more smoothly
  • Prepare for upgrading Iceberg version and Iceberg V3 support

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible
  • Add screenshots for manual tests if appropriate
  • Run test locally before making a pull request

Test commands:

  • ./mvnw -nsu -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common -am test -DfailIfNoTests=false
  • ./mvnw -nsu -pl amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common,amoro-format-mixed/amoro-mixed-flink/v1.17/amoro-mixed-flink-1.17,amoro-format-mixed/amoro-mixed-flink/v1.18/amoro-mixed-flink-1.18 -am -DskipTests package

Documentation

  • Does this pull request introduce a new feature? (yes / no)
    • no
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
    • not applicable

@github-actions github-actions bot added type:docs Improvements or additions to documentation module:mixed-flink Flink moduel for Mixed Format module:ams-server Ams server module module:mixed-trino trino module for Mixed Format type:infra type:build module:common labels Mar 17, 2026
@xxubai xxubai force-pushed the java17-experimental branch from 2ea94d3 to b6c7e1c Compare March 17, 2026 16:47
@xxubai xxubai marked this pull request as ready for review March 18, 2026 06:16
@xxubai xxubai changed the title [Draft] Support Java 17 and removing reflective/internal dependencies in Flink modules [AMORO-3853] Support Java 17 and removing reflective/internal dependencies in Flink modules Mar 18, 2026
watermarkOutput.emitPeriodicWatermark();
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reflection fallback block (lines 548-560) can be removed entirely.

SplitLocalOutput (the private inner class Flink returns from createOutputForSplit) is still registered in WatermarkOutputMultiplexer at the point releaseOutputForSplit is called — the split is not unregistered until internal.releaseOutputForSplit(splitId) returns. So calling watermarkOutput.emitPeriodicWatermark() at the reader level already covers this split: it iterates all currently-registered split outputs and computes the combined minimum watermark.

Suggested simplification:

private void emitPeriodicWatermark(SourceOutput<T> splitOutput) {
  if (splitOutput == null) {
    return;
  }

  if (splitOutput instanceof SourceOutputWithWatermarks) {
    ((SourceOutputWithWatermarks<T>) splitOutput).emitPeriodicWatermark();
    return;
  }

  // splitOutput is an internal Flink type (e.g. SplitLocalOutput from
  // ProgressiveTimestampsAndWatermarks) that does not expose
  // SourceOutputWithWatermarks publicly. The reader-level call below is
  // semantically equivalent: it iterates all registered split outputs
  // (this split is still registered until internal.releaseOutputForSplit()
  // returns) and flushes the combined periodic watermark.
  watermarkOutput.emitPeriodicWatermark();
}

This removes the setAccessible(true) call which is exactly the JDK-17 blocker this PR aims to fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good point. I removed the reflection fallback and simplified it to use watermarkOutput.emitPeriodicWatermark() directly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module module:common module:mixed-flink Flink moduel for Mixed Format module:mixed-trino trino module for Mixed Format type:build type:docs Improvements or additions to documentation type:infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement]: Support to build amoro on java 17

2 participants