wang-jiahua opened a new issue, #10972:
URL: https://github.com/apache/rocketmq/issues/10972

   ### Before Creating the Bug Report
   
   - [x] I have confirmed this should be classified as a bug rather than a 
question or enhancement.
   
   ### Runtime platform environment
   
   Linux, JDK 21, RocketMQ develop (`e348efa66`)
   
   ### Describe the Bug
   
   `TimerMessageStore#convertMessage` builds the delivered message in the wrong 
order:
   
   1. it deep-copies the properties into `msgInner`,
   2. then encodes `propertiesString` **from the original map**,
   3. and only afterwards clears `REAL_TOPIC` / `REAL_QUEUE_ID` **from the 
copy**.
   
   Two consequences:
   
   - The wire data (`propertiesString`, which is what gets stored and what 
consumers decode) still carries the internal `REAL_TOPIC` / `REAL_QUEUE_ID` 
properties, although the in-broker property map has them cleared — map and wire 
are inconsistent for the same message.
   - The RocksDB twin implementation `TimerMessageRocksDBStore#convertMessage` 
already does this correctly (clear first, then encode from the copied map), so 
the two timer store implementations deliver observably different messages for 
the same input.
   
   `EndTransactionProcessor#endMessageTransaction` also follows the correct 
clear-then-encode order.
   
   ### What Did You Expect to See?
   
   Delivered timer messages carry the same properties on the wire as in the 
property map, with internal `REAL_TOPIC` / `REAL_QUEUE_ID` removed — identical 
to the RocksDB timer store behavior.
   
   ### What Did You See Instead?
   
   Delivered timer messages from the file-based timer store retain `REAL_TOPIC` 
/ `REAL_QUEUE_ID` in the stored/transmitted properties.
   
   ### Fix
   
   Move the `setPropertiesString` encode after the clear block and encode from 
`msgInner.getProperties()` (the copied map), mirroring 
`TimerMessageRocksDBStore#convertMessage`. `TIMER_DELIVER_MS` and other timer 
metadata are not in the clearing list, so the deliveryTimestamp seen by gRPC 
consumers is unaffected.
   
   ### Verification
   
   - New regression test `testConvertMessagePropertiesStringMatchesProperties` 
(delivered message drops the internal keys on the wire and map/wire stay 
consistent; 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, 3 
interleaved trials): 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 — no regression.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to