Skip to content

fix(common): prevent multi-table commits failing with metrics enabled - #19787

Merged
danny0405 merged 1 commit into
apache:masterfrom
1fanwang:fix/local-registry-clear-race
Aug 31, 2026
Merged

fix(common): prevent multi-table commits failing with metrics enabled#19787
danny0405 merged 1 commit into
apache:masterfrom
1fanwang:fix/local-registry-clear-race

Conversation

@1fanwang

Copy link
Copy Markdown
Contributor

Describe the issue this Pull Request addresses

When one Spark driver writes several Hudi tables with metrics enabled, clearing one table's local metrics can abort another table's commit. LocalRegistry.clear() can run between containsKey() and get(), causing a concurrent metric update to fail with:

java.lang.NullPointerException: Cannot invoke "org.apache.hudi.common.metrics.Counter.add(long)" because the return value of "org.apache.hudi.common.metrics.LocalRegistry.getCounter(String)" is null

Closes #19570.

Summary and Changelog

Counter lookup now uses ConcurrentHashMap.computeIfAbsent(), so a concurrent clear cannot make it return null. The atomic lookup matches the fix proposed in #19584. This PR adds a deterministic interleaving test that reproduces the failure.

Impact

There is no public API change. Concurrent metric updates no longer fail when the local registry is cleared.

Risk Level

Low. The change replaces a compound access to an existing ConcurrentHashMap with its atomic lookup operation.

Testing

Raw failing and passing regression
$ export JAVA_HOME=$(/usr/libexec/java_home -v 17)
$ git restore --source=upstream/master --worktree hudi-io/src/main/java/org/apache/hudi/common/metrics/LocalRegistry.java
$ mvn -pl hudi-io -am -Dtest=org.apache.hudi.common.metrics.TestLocalRegistry -Dsurefire.failIfNoSpecifiedTests=false test
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
org.opentest4j.AssertionFailedError: Unexpected exception thrown: java.util.concurrent.ExecutionException: java.lang.NullPointerException: Cannot invoke "org.apache.hudi.common.metrics.Counter.add(long)" because the return value of "org.apache.hudi.common.metrics.LocalRegistry.getCounter(String)" is null
[INFO] BUILD FAILURE

$ git restore --source=HEAD --worktree hudi-io/src/main/java/org/apache/hudi/common/metrics/LocalRegistry.java
$ mvn -pl hudi-io -am -Dtest=org.apache.hudi.common.metrics.TestLocalRegistry -Dsurefire.failIfNoSpecifiedTests=false test
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
Full hudi-io suite
$ mvn -pl hudi-io -am test
[INFO] Tests run: 126, Failures: 0, Errors: 0, Skipped: 0
[INFO] hudi-io ............................................ SUCCESS
[INFO] BUILD SUCCESS

Documentation Update

None.

Contributor's checklist

  • Read through contributor's guide. Reviewed the contributor guide and repository PR template.
  • Enough context is provided in the sections above. The issue and prior implementation are linked.
  • Adequate tests were added if applicable. The deterministic regression fails on upstream/master, passes on this branch, and the full hudi-io suite passes.

@codecov-commenter

codecov-commenter commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.15%. Comparing base (01935d1) to head (f74202b).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19787      +/-   ##
============================================
- Coverage     78.16%   78.15%   -0.01%     
+ Complexity    33730    33728       -2     
============================================
  Files          2541     2541              
  Lines        141536   141534       -2     
  Branches      17138    17138              
============================================
- Hits         110634   110622      -12     
- Misses        23195    23201       +6     
- Partials       7707     7711       +4     
Components Coverage Δ
hudi-common 83.59% <ø> (-0.01%) ⬇️
hudi-client 83.14% <ø> (-0.02%) ⬇️
hudi-flink 85.65% <ø> (ø)
hudi-spark-datasource 72.69% <ø> (-0.01%) ⬇️
hudi-utilities 74.52% <ø> (-0.04%) ⬇️
hudi-cli 15.06% <ø> (ø)
hudi-hadoop 70.13% <ø> (+0.01%) ⬆️
hudi-sync 75.54% <ø> (ø)
hudi-io 79.98% <100.00%> (-0.02%) ⬇️
hudi-timeline-service 83.44% <ø> (ø)
hudi-cloud 65.81% <ø> (ø)
hudi-kafka-connect 53.20% <ø> (ø)
Flag Coverage Δ
common-and-other-modules 51.44% <100.00%> (-0.01%) ⬇️
flink-integration-tests 48.86% <100.00%> (+<0.01%) ⬆️
hadoop-mr-java-client 44.04% <100.00%> (+0.01%) ⬆️
integration-tests 13.53% <100.00%> (-0.01%) ⬇️
spark-client-hadoop-common 50.40% <100.00%> (-0.01%) ⬇️
spark-java-tests 52.19% <100.00%> (-0.02%) ⬇️
spark-scala-tests 46.70% <100.00%> (-0.03%) ⬇️
utilities 36.59% <100.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
.../org/apache/hudi/common/metrics/LocalRegistry.java 95.00% <100.00%> (-0.46%) ⬇️

... and 11 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.

@github-actions github-actions Bot added the size:M PR with lines of changes in (100, 300] label Aug 29, 2026
Signed-off-by: 1fanwang <1fannnw@gmail.com>
@1fanwang
1fanwang force-pushed the fix/local-registry-clear-race branch from 1165839 to f74202b Compare August 29, 2026 04:43
@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 0db4d3a into apache:master Aug 31, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M PR with lines of changes in (100, 300]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsynchronized LocalRegistry.clear() can NPE mid-commit and truncate the completed commit metadata file, permanently breaking the table

4 participants