Skip to content

rocketmq: avoid scanning clean TimerWheel pages during flush - #11509

Open
user-Param wants to merge 4 commits into
apache:developfrom
user-Param:develop
Open

user-Param wants to merge 4 commits into
apache:developfrom
user-Param:develop

Conversation

@user-Param

Copy link
Copy Markdown

Fixes #11508

Add per-page dirty tracking to TimerWheel.flush(). Instead of scanning
every byte in the wheel on each flush, track which logical 4 KiB pages
were modified via putSlot/reviseSlot and only scan dirty pages.

Use a claim/clear CAS pattern on an AtomicIntegerArray of page flags:

  • Writers mark pages dirty (PAGE_DIRTY) in finally blocks
  • Flush claims dirty pages (PAGE_DIRTY -> PAGE_CLAIMED), scans only those,
    then clears to clean (PAGE_CLAIMED -> PAGE_CLEAN)
  • If a writer marks dirty during processing, CAS fails and page stays dirty
  • If copy throws, page is restored to dirty for retry

Retains MappedByteBuffer.force() even when all flags are clean.
Preserves slot/file layout and snapshot backup path.

Signed-off-by: user-Param user-Param@users.noreply.github.com

melon tusk added 3 commits November 12, 2026 11:23
Add per-page dirty tracking to TimerWheel.flush(). Instead of scanning
every byte in the wheel on each flush, track which logical 4 KiB pages
were modified via putSlot/reviseSlot and only scan dirty pages.

Use a claim/clear CAS pattern on an AtomicIntegerArray of page flags:
- Writers mark pages dirty (PAGE_DIRTY) in finally blocks
- Flush claims dirty pages (PAGE_DIRTY -> PAGE_CLAIMED), scans only those,
  then clears to clean (PAGE_CLAIMED -> PAGE_CLEAN)
- If a writer marks dirty during processing, CAS fails and page stays dirty
- If copy throws, page is restored to dirty for retry

Retains MappedByteBuffer.force() even when all flags are clean.
Preserves slot/file layout and snapshot backup path.

Fixes apache#11508

Signed-off-by: melon tusk <paramjeetsingh.dev.mail@gmail.com>
IOTinyUtils.copyFile() opens the destination with FileOutputStream before
reading the source. If source and target refer to the same file (or hard
link), the destination open truncates the source first, resulting in an
empty file. Check Files.isSameFile() before opening the destination and
reject the operation with IllegalArgumentException.

Fixes apache#11500

Signed-off-by: melon tusk <paramjeetsingh.dev.mail@gmail.com>
TimerMessageStore.checkAndReviseMetrics() passes Long.MAX_VALUE (the
sentinel from TimerWheel.checkPhyPos() when no eligible slot exists)
directly to timerLog.getWholeBuffer(), causing an out-of-range warning
log. Guard against the sentinel before calling getWholeBuffer and break
the scan loop cleanly.

Fixes apache#11498

Signed-off-by: melon tusk <paramjeetsingh.dev.mail@gmail.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.79%. Comparing base (164322b) to head (445cbfb).

Files with missing lines Patch % Lines
...va/org/apache/rocketmq/store/timer/TimerWheel.java 89.47% 1 Missing and 3 partials ⚠️
.../org/apache/rocketmq/common/utils/IOTinyUtils.java 0.00% 1 Missing and 1 partial ⚠️
...apache/rocketmq/store/timer/TimerMessageStore.java 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #11509      +/-   ##
=============================================
- Coverage      49.79%   49.79%   -0.01%     
+ Complexity     14407    14406       -1     
=============================================
  Files           1391     1391              
  Lines         103381   103404      +23     
  Branches       13529    13535       +6     
=============================================
+ Hits           51477    51485       +8     
- Misses         45686    45698      +12     
- Partials        6218     6221       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

Add test for IOTinyUtils.copyFile to verify that copying a file to itself
throws IllegalArgumentException. This covers the new check added in
GSoC PR apache#11509 that prevents copying a file to itself.

Fixes apache#11509

Signed-off-by: Paramjeet Singh <user-Param@users.noreply.github.com>
@user-Param

Copy link
Copy Markdown
Author

@RongtongJin @shijianxk Could you please review this PR when you have a chance? Thank you!

@user-Param

Copy link
Copy Markdown
Author

@shijia.wxr Could you please review this PR when you have a chance? Thank you!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] Avoid scanning clean TimerWheel pages during flush

2 participants