[ISSUE #10972] Encode timer message propertiesString after internal properties are cleared

[wang-jiahua]

Which Issue(s) This PR Fixes

Fixes #10972

Brief Description

TimerMessageStore#convertMessage encoded propertiesString from the original property map and only afterwards cleared REAL_TOPIC / REAL_QUEUE_ID from the deep copy. As a result the delivered timer message carried the internal properties on the wire while the broker-side map had them cleared, and the file-based timer store observably diverged from TimerMessageRocksDBStore#convertMessage, which already clears first and encodes from the copied map.

This PR moves the encode after the clear block and encodes from msgInner.getProperties(), mirroring the RocksDB implementation. It also removes the map/wire mismatch where the deep copy was made but the encode still read the original map. TIMER_DELIVER_MS and other timer metadata are not in the clearing list, so the deliveryTimestamp exposed to gRPC consumers is unchanged.

How Did You Test This Change?

• New regression test testConvertMessagePropertiesStringMatchesProperties: the delivered message drops REAL_TOPIC/REAL_QUEUE_ID from the wire and propertiesString equals the re-encoded map; the rolled message keeps them. TimerMessageStoreTest passes 11/11.
• 4-node cluster A/B (file-based timer store, benchmark.timer.TimerProducer 64 threads x 40 slots x 250 msgs/slot, 1 KiB, per arm clean store + page cache drop, 3 interleaved trials; the 2-minute first-slot offset isolates GC sampling to the pure delivery phase): every arm delivered 639,990/640,000 (report-sampling rounding) with zero send failures; delivery-phase young GC base 2/2/2 vs patch 1/2/2 — correctness intact, no regression.

[Copilot]

Pull request overview

Fixes a bug in the file-based timer store message conversion so propertiesString is encoded after internal properties (REAL_TOPIC / REAL_QUEUE_ID) are cleared, keeping the wire representation consistent with the broker-side properties map and matching the RocksDB timer store behavior.

Changes:

• Update TimerMessageStore#convertMessage to clear internal properties first, then encode propertiesString from the copied/cleared properties map.
• Add a regression test to ensure delivered timer messages do not carry internal properties on the wire and that propertiesString matches the re-encoded properties map.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
store/src/main/java/org/apache/rocketmq/store/timer/TimerMessageStore.java	Encodes propertiesString after clearing internal timer properties to keep map/wire consistent.
store/src/test/java/org/apache/rocketmq/store/timer/TimerMessageStoreTest.java	Adds regression coverage verifying delivered vs rolled timer message property handling and map/wire consistency.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.50%. Comparing base (e348efa) to head (20b4dc7).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10974      +/-   ##
=============================================
- Coverage      48.58%   48.50%   -0.08%     
+ Complexity     13676    13655      -21     
=============================================
  Files           1381     1381              
  Lines         101475   101475              
  Branches       13190    13190              
=============================================
- Hits           49299    49221      -78     
- Misses         46174    46245      +71     
- Partials        6002     6009       +7     

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

[RockteMQ-AI]

Summary

This PR fixes a correctness bug in TimerMessageStore.convertMessage where propertiesString was encoded before internal properties (PROPERTY_REAL_TOPIC, PROPERTY_REAL_QUEUE_ID) were cleared from the property map. This caused the wire data to contain stale properties that didn't match the actual property map.

The fix moves setPropertiesString to after the property clearing logic, ensuring consistency between the encoded string and the property map. The comment correctly references TimerMessageRocksDBStore#convertMessage as the aligned implementation.

The test verifies both scenarios: delivered messages (properties cleared) and rolled messages (properties retained), confirming that propertiesString matches messageProperties2String(properties) in both cases.

Important bug fix — the wire format and property map must stay consistent. LGTM.

---

Automated review by github-manager-bot