Skip to content

test(common): run 10 instead of 50 validations per thread in TestBootstrapIndex#testBootstrapIndexConcurrent - #20358

Closed
databird07 wants to merge 1 commit into
apache:masterfrom
databird07:test/bootstrap-index-fewer-validations
Closed

databird07 wants to merge 1 commit into
apache:masterfrom
databird07:test/bootstrap-index-fewer-validations

Conversation

@databird07

Copy link
Copy Markdown
Contributor

Describe the issue this Pull Request addresses

testBootstrapIndexConcurrent took ~85s in the UT - hudi-hadoop-common GitHub step. Almost all of that time went to repeating the same full-index validation 50 times per thread after the readers were already open.

Summary and Changelog

  • numRequestsPerThread 50 -> 10. Still 20 concurrent threads, each with its own HFileBootstrapIndex reader (200 full validations instead of 1,000).
  • Sharing one reader across threads was considered and rejected: the reader cursor isn't thread-safe, and production code opens one reader per use.

Impact

Test-only. Local median 50.2s -> 14.5s over 20 runs each, 20/20 passing. JaCoCo coverage is identical (3,128 lines / 489 branches). Measured locally on JDK 17 with 4 pinned CPUs and JaCoCo on; full numbers are in the commit message.

Risk Level

low. Fewer iterations lowers the per-run chance of hitting a rare race in concurrent reader opening.

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

…strapIndex#testBootstrapIndexConcurrent

testBootstrapIndexConcurrent starts 20 threads that each open their own
HFileBootstrapIndex reader over the same index files and validate the full
bootstrap mapping 50 times (1,000 full validations). It took ~85s of the
"UT - hudi-hadoop-common" GitHub step, almost all of it repeating the same
reads after the readers were already open.

Change numRequestsPerThread from 50 to 10. The test still runs 20 threads
concurrently, each with an independent reader (200 full validations), so it
still exercises concurrent reader creation and lookups against the same
files. Sharing one reader across threads was considered and rejected:
HFileBootstrapIndexReader only synchronizes lazy reader creation, while
getAllKeys / getSourceFileMappingFor* move a stateful HFile cursor without
locking, and production code opens one reader per use.

Measured locally (JDK 17, 4 pinned CPUs, JaCoCo on), 20 runs each:
- before: 20/20 passed (4 tests), median 50.2s, p90 52.6s
- after:  20/20 passed (4 tests), median 14.5s, p90 15.3s
- JaCoCo across all hudi modules: 3,128 lines / 489 branches covered
  before and after, no line lost or gained.

Tradeoff: fewer iterations per run lowers the per-run chance of hitting a
rare race in concurrent reader opening.
@github-actions github-actions Bot added the size:XS PR with lines of changes in <= 10 label Oct 16, 2026

@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 trimming this! This test-only change drops the per-thread validation loop in testBootstrapIndexConcurrent from 50 to 10 iterations and keeps all 20 concurrent threads, each with its own reader. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@databird07

Copy link
Copy Markdown
Contributor Author

Combined into #20364 so the total impact can be reviewed in one place; this commit is included there unchanged. Closing.

@databird07 databird07 closed this Oct 16, 2026
@codecov-commenter

codecov-commenter commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.11%. Comparing base (c167f7b) to head (c099cdd).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #20358      +/-   ##
============================================
- Coverage     80.73%   78.11%   -2.63%     
+ Complexity    35201    34015    -1186     
============================================
  Files          2553     2553              
  Lines        143629   143629              
  Branches      17504    17504              
============================================
- Hits         115964   112192    -3772     
- Misses        19703    23258    +3555     
- Partials       7962     8179     +217     
Components Coverage Δ
hudi-common 82.48% <ø> (-1.68%) ⬇️
hudi-client 79.60% <ø> (-4.36%) ⬇️
hudi-flink 86.08% <ø> (+<0.01%) ⬆️
hudi-spark-datasource 67.19% <ø> (-7.00%) ⬇️
hudi-utilities 78.18% <ø> (-0.01%) ⬇️
hudi-cli 70.80% <ø> (ø)
hudi-hadoop 69.06% <ø> (-2.13%) ⬇️
hudi-sync 76.78% <ø> (ø)
hudi-io 81.43% <ø> (-0.05%) ⬇️
hudi-timeline-service 78.53% <ø> (-4.88%) ⬇️
hudi-cloud 81.00% <ø> (ø)
hudi-kafka-connect 58.21% <ø> (+0.30%) ⬆️
Flag Coverage Δ
common-and-other-modules 52.63% <ø> (ø)
flink-integration-tests 49.64% <ø> (+<0.01%) ⬆️
hadoop-mr-java-client 43.94% <ø> (+<0.01%) ⬆️
integration-tests 13.44% <ø> (ø)
spark-client-hadoop-common 38.67% <ø> (-0.01%) ⬇️
spark-java-tests 32.14% <ø> (-20.65%) ⬇️
utilities 36.88% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 379 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.

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

Labels

size:XS PR with lines of changes in <= 10

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants