Skip to content

[fix][test]fix flaky MessagePayloadProcessorTest.testDefaultProcessor#24710

Merged
codelipenghui merged 1 commit intoapache:masterfrom
3pacccccc:fixFlakyRefCount
Sep 8, 2025
Merged

[fix][test]fix flaky MessagePayloadProcessorTest.testDefaultProcessor#24710
codelipenghui merged 1 commit intoapache:masterfrom
3pacccccc:fixFlakyRefCount

Conversation

@3pacccccc
Copy link
Copy Markdown
Contributor

Fixes #24709

Motivation

The flakiness occurred because Assert.assertEquals(processor.getTotalRefCnt(), 2 * numBatches)1 was sometimes executed before totalRefCnt += ((MessagePayloadImpl) payload).getByteBuf().refCnt(). 2 This race condition led to inconsistent test results. By updating the reference count first, we ensure the assertion always evaluates correctly.

Modifications

Moved totalRefCnt += ((MessagePayloadImpl) payload).getByteBuf().refCnt()2 above the MessagePayloadProcessor.DEFAULT.process3 call.

Verifying this change

  • Make sure that the change passes the CI checks.

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: 3pacccccc#25

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 5, 2025
@Technoboy- Technoboy- added this to the 4.2.0 milestone Sep 8, 2025
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.17%. Comparing base (77b34cc) to head (f5176e5).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24710      +/-   ##
============================================
+ Coverage     74.12%   74.17%   +0.04%     
- Complexity    33068    33476     +408     
============================================
  Files          1895     1895              
  Lines        147979   147979              
  Branches      17137    17137              
============================================
+ Hits         109693   109759      +66     
+ Misses        29524    29463      -61     
+ Partials       8762     8757       -5     
Flag Coverage Δ
inttests 26.25% <ø> (-0.34%) ⬇️
systests 22.65% <ø> (-0.03%) ⬇️
unittests 73.68% <ø> (+0.04%) ⬆️

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

@codelipenghui codelipenghui merged commit 465281c into apache:master Sep 8, 2025
52 checks passed
@3pacccccc 3pacccccc deleted the fixFlakyRefCount branch November 6, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky-test: MessagePayloadProcessorTest.testDefaultProcessor

4 participants