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

   ### Before Creating the Enhancement Request
   
   - [x] I have confirmed that this should be classified as an enhancement 
rather than a bug/feature.
   
   ### Summary
   
   Serialize POP checkpoint/ack messages with `JSON.toJSONBytes` and 
deserialize them with `JSON.parseObject(byte[], ...)` instead of going through 
an intermediate JSON `String`.
   
   ### Motivation
   
   Every POP checkpoint and ack that reaches the revive topic is serialized as 
`JSON.toJSONString(x).getBytes(UTF_8)` — a full `String` (with its internal 
array) plus a second `byte[]` copy per record — in five call sites: 
`PopMessageProcessor#buildCkMsg`, `PopBufferMergeService#putAckToStore` / 
`putBatchAckToStore`, `AckMessageProcessor`, and `ChangeInvisibleTimeProcessor` 
(ack + re-put CK). On the read side, `PopReviveService#scanReviveQueue` first 
materializes `new String(body)` and then parses it, for CK, ack, and batch-ack 
records alike.
   
   The newer popkv implementation already does this directly: 
`PopConsumerRecord` uses `JSON.toJSONBytes(this)` / `JSON.parseObject(body, 
...)`.
   
   ### Solution
   
   - Replace the six encode sites with `JSON.toJSONBytes(x)`.
   - Parse revive records straight from `messageExt.getBody()`; the raw string 
is now built only inside the `enablePopLog` branch that logs it.
   
   The stored bytes are unchanged: fastjson2's `toJSONBytes` writes the same 
UTF-8 bytes as `toJSONString().getBytes(UTF_8)`. New tests assert byte-for-byte 
equality (including non-ASCII field values) and byte-array round-trips for 
`PopCheckPoint`, `AckMsg`, and `BatchAckMsg`.
   
   ### Verification
   
   - New equivalence/round-trip tests: `PopCheckPointTest`, plus new cases in 
`AckMsgTest` / `BatchAckMsgTest` — 5/5 pass.
   - Single-class clean runs of the touched broker tests: 
`AckMessageProcessorTest` 8/8, `ChangeInvisibleTimeProcessorTest` 9/9, 
`PopMessageProcessorTest` 8/8, `PopBufferMergeServiceTest` 4/4; 
`PopReviveServiceTest` matches the develop baseline exactly (one pre-existing 
failure, identical with and without this change).
   - 4-node cluster A/B in POP mode (`mqadmin setConsumeMode -m POP`, producer 
64 threads + consumer 20 threads, consume TPS steady at ~150k, pop path 
confirmed active, 3 interleaved trials, broker jar swapped per arm): broker 
young GC per million consumed msgs 2.67/2.68/2.66 (base) vs 2.66/2.63/2.79 
(patch) — parity, no regression. The saving itself (one string + one copy per 
CK/ack) is below GC-count resolution at this load; this is a cleanup-level 
optimization consistent with the popkv precedent.
   


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