Skip to content

fix(flink): improve clustering resource cleanup and coverage - #19400

Merged
danny0405 merged 1 commit into
apache:masterfrom
cshuo:flink-clustering-compaction-coverage
Jul 30, 2026
Merged

fix(flink): improve clustering resource cleanup and coverage#19400
danny0405 merged 1 commit into
apache:masterfrom
cshuo:flink-clustering-compaction-coverage

Conversation

@cshuo

@cshuo cshuo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Describe the issue this Pull Request addresses

Closes #19397.

The Flink clustering and compaction operators and the consistent-bucket update strategy had low or missing unit-test coverage. The standalone job drivers remain integration-test-only and are tracked separately by ENG-44609 because that coverage is not currently uploaded.

Summary and Changelog

  • Add Flink operator-harness tests for synchronous and asynchronous clustering execution.
  • Cover clustering reads from base files and file slices with log files.
  • Exercise the clustering sort path and emitted commit events.
  • Cover clustering commit success, failure rollback, missing plans, write errors, and inline cleaning.
  • Cover consistent-bucket initialization, pending clustering refresh, dual-write routing, and file ID patching.
  • Cover compaction plan emission and missing pending compaction instants.

Local JaCoCo line coverage from the targeted suite:

Class Before After
ClusteringOperator 49% 91.5%
ClusteringCommitSink 64% 91.5%
FlinkConsistentBucketUpdateStrategy 0% 100.0%
CompactionPlanSourceFunction 0% 100.0%

Impact

No public API or user-facing behavior changes. This PR only adds tests.

Risk Level

Low. Production code is unchanged.

Validation

The targeted Maven and JaCoCo-instrumented test runs completed successfully:

Tests run: 13, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Checkstyle and RAT also passed.

Documentation Update

None.

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

@cshuo
cshuo force-pushed the flink-clustering-compaction-coverage branch 2 times, most recently from 06bd669 to f239f45 Compare July 29, 2026 02:22
@cshuo cshuo changed the title [MINOR] Improve Flink clustering and compaction test coverage test(flink): improve clustering and compaction coverage Jul 29, 2026
@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.64%. Comparing base (d56e674) to head (0b65db5).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19400      +/-   ##
============================================
+ Coverage     72.19%   73.64%   +1.44%     
- Complexity    32595    32815     +220     
============================================
  Files          2561     2574      +13     
  Lines        148328   146111    -2217     
  Branches      18654    18147     -507     
============================================
+ Hits         107087   107605     +518     
+ Misses        32774    30416    -2358     
+ Partials       8467     8090     -377     
Components Coverage Δ
hudi-common 81.61% <100.00%> (+0.20%) ⬆️
hudi-client 80.35% <ø> (-1.22%) ⬇️
hudi-flink 79.12% <100.00%> (+0.49%) ⬆️
hudi-spark-datasource 64.38% <ø> (+5.71%) ⬆️
hudi-utilities 71.18% <ø> (+0.76%) ⬆️
hudi-cli 15.32% <ø> (ø)
hudi-hadoop 63.49% <ø> (ø)
hudi-sync 69.14% <ø> (-1.50%) ⬇️
hudi-io 79.57% <ø> (ø)
hudi-timeline-service 83.83% <ø> (+0.39%) ⬆️
hudi-cloud 64.00% <ø> (ø)
hudi-kafka-connect 53.20% <ø> (ø)
Flag Coverage Δ
common-and-other-modules 47.90% <100.00%> (+1.20%) ⬆️
flink-integration-tests 47.77% <100.00%> (+0.06%) ⬆️
hadoop-mr-java-client 43.37% <0.00%> (+0.04%) ⬆️
spark-client-hadoop-common 48.71% <0.00%> (+0.01%) ⬆️
spark-java-tests 51.34% <100.00%> (+2.01%) ⬆️
spark-scala-tests 45.94% <0.00%> (+1.08%) ⬆️
utilities 36.65% <0.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...til/collection/CloseableConcatenatingIterator.java 100.00% <100.00%> (ø)
.../apache/hudi/sink/bulk/BulkInsertWriterHelper.java 73.46% <ø> (ø)
...pache/hudi/sink/clustering/ClusteringOperator.java 92.36% <100.00%> (+13.90%) ⬆️

... and 78 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cshuo
cshuo force-pushed the flink-clustering-compaction-coverage branch from f239f45 to b3cde65 Compare July 29, 2026 03:02
@cshuo
cshuo marked this pull request as ready for review July 29, 2026 03:08

@hudi-agent hudi-agent left a comment

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.

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR adds Flink operator-harness and unit tests for the clustering operator, clustering commit sink, consistent-bucket update strategy, and compaction plan source function, with no production code changes. 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. One minor naming nit in the test helper; the rest of the code is clean and well-structured.

cc @yihua

@github-actions github-actions Bot added the size:L PR with lines of changes in (300, 1000] label Jul 29, 2026

@danny0405 danny0405 left a comment

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 success paths look covered, but the new clustering tests expose an exception-path resource leak that should be addressed.

@cshuo
cshuo force-pushed the flink-clustering-compaction-coverage branch from b3cde65 to ab583a6 Compare July 29, 2026 07:18
Close clustering readers, sorters, and writers on failures to prevent resource leaks in long-running Flink tasks.

Expand coverage for the clustering operator, clustering commit sink, compaction plan source function, and consistent bucket update strategy.

Addresses review comment 3670917473.
@cshuo
cshuo force-pushed the flink-clustering-compaction-coverage branch from ab583a6 to 0b65db5 Compare July 29, 2026 07:22
@cshuo cshuo changed the title test(flink): improve clustering and compaction coverage fix(flink): improve clustering resource cleanup and coverage Jul 29, 2026
@github-actions github-actions Bot added size:XL PR with lines of changes > 1000 and removed size:L PR with lines of changes in (300, 1000] labels Jul 29, 2026
@hudi-bot

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@danny0405
danny0405 merged commit 45dcf1e into apache:master Jul 30, 2026
75 of 77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL PR with lines of changes > 1000

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Flink] Improve Flink clustering and compaction unit test coverage

5 participants