Skip to content

fix(flink): remove pre-seed in metrics in GlobalRecordLevelIndexBackend and RecordLevelIndexBackend#18875

Merged
danny0405 merged 1 commit into
apache:masterfrom
skywalker0618:shihuanl/remove_pre_seed_in_rli_metrics
May 29, 2026
Merged

fix(flink): remove pre-seed in metrics in GlobalRecordLevelIndexBackend and RecordLevelIndexBackend#18875
danny0405 merged 1 commit into
apache:masterfrom
skywalker0618:shihuanl/remove_pre_seed_in_rli_metrics

Conversation

@skywalker0618
Copy link
Copy Markdown
Contributor

@skywalker0618 skywalker0618 commented May 28, 2026

Describe the issue this Pull Request addresses

The pre-seed logic in metrics in GlobalRecordLevelIndexBackend and RecordLevelIndexBackend are not needed by production code and they only existed for test purpose. This PR removes this unnecessary step and fix all test cases.

Fixes #18734

Summary and Changelog

Remove pre-seed in metrics in GlobalRecordLevelIndexBackend and RecordLevelIndexBackend and fix test cases

Impact

None

Risk Level

None. Production code doesn't need pre-seed.

Documentation Update

N/A

Contributor's checklist

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

Copy link
Copy Markdown
Contributor

@hudi-agent hudi-agent left a comment

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 the cleanup! This PR removes the UnregisteredMetricsGroup pre-seed from both index backend constructors and updates tests to register metrics explicitly when they exercise the metric-touching paths. I traced the production callers (BucketAssignFunction.initializeState and DynamicBucketAssignFunction.initializeState) and confirmed both invoke registerMetrics(...) immediately after construction, before any get/update/bootstrapPartition call that would dereference metrics, so removing the pre-seed is safe for production paths. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@github-actions github-actions Bot added the size:S PR with lines of changes in (10, 100] label May 28, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.81%. Comparing base (ff72186) to head (60c93b0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #18875   +/-   ##
=========================================
  Coverage     68.81%   68.81%           
- Complexity    29142    29144    +2     
=========================================
  Files          2515     2515           
  Lines        139931   139929    -2     
  Branches      17187    17187           
=========================================
+ Hits          96288    96296    +8     
+ Misses        35864    35854   -10     
  Partials       7779     7779           
Flag Coverage Δ
common-and-other-modules 44.32% <ø> (-0.01%) ⬇️
hadoop-mr-java-client 44.90% <ø> (-0.01%) ⬇️
spark-client-hadoop-common 48.22% <ø> (-0.01%) ⬇️
spark-java-tests 49.37% <ø> (-0.03%) ⬇️
spark-scala-tests 45.27% <ø> (-0.01%) ⬇️
utilities 37.46% <ø> (+0.02%) ⬆️

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

Files with missing lines Coverage Δ
...rtitioner/index/GlobalRecordLevelIndexBackend.java 90.16% <ø> (-0.16%) ⬇️
...ink/partitioner/index/RecordLevelIndexBackend.java 56.57% <ø> (-0.29%) ⬇️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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 fc85e3e into apache:master May 29, 2026
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S PR with lines of changes in (10, 100]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add metrics for remote RLI lookup time cost in BucketAssign op

5 participants