镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

[ISSUE #11037] Encode delay message propertiesString after internal properties are cleared - #11038

Closed
unbridled-41 wants to merge 1 commit into
apache:developfrom
unbridled-41:fix/schedule-message-timeup-properties
Closed

unbridled-41 wants to merge 1 commit into
apache:developfrom
unbridled-41:fix/schedule-message-timeup-properties

Conversation

@unbridled-41

@unbridled-41 unbridled-41 commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Evidence

ScheduleMessageService#messageTimeUp encodes the delivered delay message's propertiesString before the internal properties are cleared (encode at the top of the method, clearProperty for DELAY_TIME_LEVEL / TIMER_DELIVER_MS / TIMER_DELAY_SEC afterwards). propertiesString is what gets persisted in the commitlog and decoded by consumers, so every delay-level message still carries those internal properties on the wire while the broker-side property map no longer does — the two disagree, and SQL92 filtering / user code see stale internal properties.

This is the exact defect class fixed for the timer-wheel path by #10972 (commit e533b66, TimerMessageStore#convertMessage now encodes after clearing). The identical pattern in messageTimeUp predates it (the 2022-era unmerged PR #4190 attempted the same cleanup) and was not covered.

Regression test ScheduleMessageServiceTest#testMessageTimeUpPropertiesStringMatchesProperties fails before the fix (propertiesString contains DELAY_TIME_LEVEL/TIMER_DELIVER_MS and differs from re-encoding the cleared map) and passes after.

Root cause / Fix

Move the setPropertiesString(...) call after the clearProperty(...) calls and encode msgInner.getProperties() — mirroring the upstream-accepted fix in TimerMessageStore#convertMessage.

Priority

PRIORITY = 74:影响 28(延迟消息是核心特性,每条延迟投递消息的线上属性与 broker 端属性不一致,内部属性泄漏给消费者)+ 波及范围 12(单方法,全部延迟消息)+ 可复现性 20(确定性单元测试)+ 维护价值 14(与上游刚合并的 #10972 同类修复,先例明确)。FIX_CONFIDENCE = 95(一行位置调整,镜像上游已接受的同型修复)。

Tests

  • mvn -pl broker test -Dtest=ScheduleMessageServiceTest#testMessageTimeUpPropertiesStringMatchesProperties
    • before fix (commit ff8f6f7 + test only): Tests run: 1, Failures: 1 (AssertionError at line 298)
    • after fix: Tests run: 1, Failures: 0
  • mvn -pl broker test -Dtest=ScheduleMessageServiceTest: Tests run: 5, Failures: 0, Errors: 0

Risk

Low. The only behavior change is that the persisted propertiesString of delivered delay messages no longer contains the three internal keys — this is what the property map already said since those lines existed, and matches the timer-path fix accepted in #10972. Consumers relying on the leaked internal keys would see them disappear, which is the documented intended behavior.

Closes #11037

…rnal properties are cleared

ScheduleMessageService#messageTimeUp encoded propertiesString before
clearing DELAY_TIME_LEVEL / TIMER_DELIVER_MS / TIMER_DELAY_SEC, so the
persisted wire data still carried the internal properties while the
broker-side property map no longer did. Encode after the clears,
aligning with TimerMessageStore#convertMessage (ISSUE apache#10972).
@codecov-commenter

codecov-commenter commented Sep 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.75%. Comparing base (ff8f6f7) to head (f7752f5).
⚠️ Report is 46 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #11038      +/-   ##
=============================================
- Coverage      48.86%   48.75%   -0.11%     
+ Complexity     13797    13765      -32     
=============================================
  Files           1381     1381              
  Lines         101574   101574              
  Branches       13213    13213              
=============================================
- Hits           49638    49527     -111     
- Misses         45924    46022      +98     
- Partials        6012     6025      +13     

☔ 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.

@unbridled-41
unbridled-41 marked this pull request as ready for review September 5, 2026 11:32
@unbridled-41

Copy link
Copy Markdown
Contributor Author

Evidence chain (audit 2026-09-05):

Before the fix — base ff8f6f7 + regression test only (production change stashed):

mvn -pl broker test -Dtest=ScheduleMessageServiceTest#testMessageTimeUpPropertiesStringMatchesProperties
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
java.lang.AssertionError
    at ...ScheduleMessageServiceTest.testMessageTimeUpPropertiesStringMatchesProperties(ScheduleMessageServiceTest.java:298)

The first failing assertion is assertFalse(delivered.getPropertiesString().contains(MessageConst.PROPERTY_DELAY_TIME_LEVEL)) — pre-fix the persisted wire string still carries DELAY_TIME_LEVEL/TIMER_DELIVER_MS and differs from re-encoding the cleared property map.

After the fix — branch tip f7752f5, re-measured 2026-09-05:

mvn -pl broker test -Dtest=ScheduleMessageServiceTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0

Precedent anchor: the identical defect class in TimerMessageStore#convertMessage was fixed upstream in e533b66 (ISSUE #10972, encode after clearProperty, encode msgInner.getProperties()); the schedule path was not covered there, and the 2022-era PR #4190 that attempted it was closed unmerged.

@RockteMQ-AI RockteMQ-AI 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.

Summary

Fixes delay message encoding to regenerate propertiesString after internal properties are cleared, ensuring consistency between the property map and wire data. The test verifies both the encoded bytes and the decoded property map.

LGTM.


Automated review by github-manager-bot

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.

[Bug] Delay messages are delivered with internal properties leaked into propertiesString (ScheduleMessageService#messageTimeUp)

3 participants