You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "wang-jiahua (via GitHub)" <gi...@apache.org> on 2026/06/21 01:49:22 UTC
[PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
wang-jiahua opened a new pull request, #10535:
URL: https://github.com/apache/rocketmq/pull/10535
<!-- Please make sure the target branch is right. In most case, the target branch should be `develop`. -->
### Which Issue(s) This PR Fixes
<!-- Please ensure that the related issue has already been created, and [link this pull request to that issue using keywords](<https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword>) to ensure automatic closure. -->
Fixes #10533
### Brief Description
Implement `FastCodesHeader.encode()/decode()` for `SendMessageRequestHeader` and `SendMessageResponseHeader` to eliminate reflection-based header serialization on the send message hot path.
1. **`SendMessageRequestHeader`** — Implement `encode()` using `writeIfNotNull` for 14 fields, and `decode()` using `getAndCheckNotNull`. This replaces the reflection-based `makeCustomHeaderToNet()` path which uses `Field.get()` + `toString()` for every field on every send.
2. **`SendMessageResponseHeader`** — Implement `encode()` and `decode()` for 5 response fields.
3. **`SendMessageRequestHeaderV2`** — Fix `decode()` signature from `Map<String, String>` to `HashMap<String, String>` to match the `FastCodesHeader` interface.
### How Did You Test This Change?
1. **Unit tests**: All 16 `SendMessageProcessorTest` tests pass.
2. **Compilation**: `remoting` + `broker` modules compile cleanly on JDK 21.
3. **Commercial compatibility**: `YitianSendMessageRequestHeader extends SendMessageRequestHeader` (subclass, unaffected). `YitianSendMessageResponseHeader implements FastCodesHeader` with its own `encode()/decode()` (independent implementation, unaffected).
4. **Microbenchmark** (R1 encode/decode bench): `writeIfNotNull` + `writeDecimalInt` is 25% faster than `value.toString()` + `writeStr` for encoding; `readStr` with single-byte cache is 56% faster for decoding.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
Posted by "wang-jiahua (via GitHub)" <gi...@apache.org>.
wang-jiahua commented on PR #10535:
URL: https://github.com/apache/rocketmq/pull/10535#issuecomment-4766521090
## Server Benchmark Results (2026-06-22)
**Environment**: 8C30G broker, Dragonwell JDK 21, 1KB msg, 128 queues, sync mode, 256 threads, 20s warmup + 30s collect
### Producer-only (steady state, lines 3-5)
| Test | TPS | Avg RT | GC | vs Baseline |
|------|-----|--------|----|-------------|
| Baseline | 217k | 1.18ms | 354 | — |
| #10535 | 235k | 1.09ms | 332 | **+8.3% TPS, -7.6% RT** ✅ |
Send header FastCodesHeader encode/decode eliminates reflection-based `makeCustomHeaderToNet()` on every Send request, yielding the largest TPS improvement among all PRs.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
Posted by "wang-jiahua (via GitHub)" <gi...@apache.org>.
wang-jiahua commented on code in PR #10535:
URL: https://github.com/apache/rocketmq/pull/10535#discussion_r3447809629
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeader.java:
##########
@@ -183,22 +191,140 @@ public void setBatch(Boolean batch) {
}
public static SendMessageRequestHeader parseRequestHeader(RemotingCommand request) throws RemotingCommandException {
- SendMessageRequestHeaderV2 requestHeaderV2 = null;
- SendMessageRequestHeader requestHeader = null;
- switch (request.getCode()) {
- case RequestCode.SEND_BATCH_MESSAGE:
- case RequestCode.SEND_MESSAGE_V2:
- requestHeaderV2 = request.decodeCommandCustomHeader(SendMessageRequestHeaderV2.class);
- case RequestCode.SEND_MESSAGE:
- if (null == requestHeaderV2) {
- requestHeader = request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
- } else {
- requestHeader = SendMessageRequestHeaderV2.createSendMessageRequestHeaderV1(requestHeaderV2);
- }
- default:
- break;
- }
- return requestHeader;
+ return request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
+ }
+
+ @Override
+ public void encode(ByteBuf out) {
+ writeIfNotNull(out, "a", producerGroup);
+ writeIfNotNull(out, "b", topic);
+ writeIfNotNull(out, "c", defaultTopic);
+ writeIfNotNull(out, "d", defaultTopicQueueNums);
+ writeIfNotNull(out, "e", queueId);
+ writeIfNotNull(out, "f", sysFlag);
+ writeIfNotNull(out, "g", bornTimestamp);
+ writeIfNotNull(out, "h", flag);
+ writeIfNotNull(out, "i", properties);
+ writeIfNotNull(out, "j", reconsumeTimes);
+ writeIfNotNull(out, "k", unitMode);
+ writeIfNotNull(out, "l", maxReconsumeTimes);
+ writeIfNotNull(out, "m", batch);
+ writeIfNotNull(out, "n", getBname());
+ }
Review Comment:
@/tmp/r2-t1.txt
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeader.java:
##########
@@ -183,22 +191,140 @@ public void setBatch(Boolean batch) {
}
public static SendMessageRequestHeader parseRequestHeader(RemotingCommand request) throws RemotingCommandException {
- SendMessageRequestHeaderV2 requestHeaderV2 = null;
- SendMessageRequestHeader requestHeader = null;
- switch (request.getCode()) {
- case RequestCode.SEND_BATCH_MESSAGE:
- case RequestCode.SEND_MESSAGE_V2:
- requestHeaderV2 = request.decodeCommandCustomHeader(SendMessageRequestHeaderV2.class);
- case RequestCode.SEND_MESSAGE:
- if (null == requestHeaderV2) {
- requestHeader = request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
- } else {
- requestHeader = SendMessageRequestHeaderV2.createSendMessageRequestHeaderV1(requestHeaderV2);
- }
- default:
- break;
- }
- return requestHeader;
+ return request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
+ }
+
+ @Override
+ public void encode(ByteBuf out) {
+ writeIfNotNull(out, "a", producerGroup);
+ writeIfNotNull(out, "b", topic);
+ writeIfNotNull(out, "c", defaultTopic);
+ writeIfNotNull(out, "d", defaultTopicQueueNums);
+ writeIfNotNull(out, "e", queueId);
+ writeIfNotNull(out, "f", sysFlag);
+ writeIfNotNull(out, "g", bornTimestamp);
+ writeIfNotNull(out, "h", flag);
+ writeIfNotNull(out, "i", properties);
+ writeIfNotNull(out, "j", reconsumeTimes);
+ writeIfNotNull(out, "k", unitMode);
+ writeIfNotNull(out, "l", maxReconsumeTimes);
+ writeIfNotNull(out, "m", batch);
+ writeIfNotNull(out, "n", getBname());
+ }
Review Comment:
@/tmp/r2-t2.txt
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeaderV2.java:
##########
@@ -20,6 +20,7 @@
import com.google.common.base.MoreObjects;
import io.netty.buffer.ByteBuf;
import java.util.HashMap;
+import java.util.Map;
Review Comment:
@/tmp/r2-fix.txt
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #10535:
URL: https://github.com/apache/rocketmq/pull/10535#discussion_r3447778765
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeader.java:
##########
@@ -183,22 +191,140 @@ public void setBatch(Boolean batch) {
}
public static SendMessageRequestHeader parseRequestHeader(RemotingCommand request) throws RemotingCommandException {
- SendMessageRequestHeaderV2 requestHeaderV2 = null;
- SendMessageRequestHeader requestHeader = null;
- switch (request.getCode()) {
- case RequestCode.SEND_BATCH_MESSAGE:
- case RequestCode.SEND_MESSAGE_V2:
- requestHeaderV2 = request.decodeCommandCustomHeader(SendMessageRequestHeaderV2.class);
- case RequestCode.SEND_MESSAGE:
- if (null == requestHeaderV2) {
- requestHeader = request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
- } else {
- requestHeader = SendMessageRequestHeaderV2.createSendMessageRequestHeaderV1(requestHeaderV2);
- }
- default:
- break;
- }
- return requestHeader;
+ return request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
+ }
+
+ @Override
+ public void encode(ByteBuf out) {
+ writeIfNotNull(out, "a", producerGroup);
+ writeIfNotNull(out, "b", topic);
+ writeIfNotNull(out, "c", defaultTopic);
+ writeIfNotNull(out, "d", defaultTopicQueueNums);
+ writeIfNotNull(out, "e", queueId);
+ writeIfNotNull(out, "f", sysFlag);
+ writeIfNotNull(out, "g", bornTimestamp);
+ writeIfNotNull(out, "h", flag);
+ writeIfNotNull(out, "i", properties);
+ writeIfNotNull(out, "j", reconsumeTimes);
+ writeIfNotNull(out, "k", unitMode);
+ writeIfNotNull(out, "l", maxReconsumeTimes);
+ writeIfNotNull(out, "m", batch);
+ writeIfNotNull(out, "n", getBname());
+ }
+
+ @Override
+ public void decode(HashMap<String, String> fields) throws RemotingCommandException {
+ String str = fields.get("a");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "producerGroup");
+ }
+ if (str != null) {
+ this.producerGroup = str;
+ }
+
+ str = fields.get("b");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "topic");
+ }
+ if (str != null) {
+ this.topic = str;
+ }
+
+ str = fields.get("c");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "defaultTopic");
+ }
+ if (str != null) {
+ this.defaultTopic = str;
+ }
+
+ str = fields.get("d");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "defaultTopicQueueNums");
+ }
+ if (str != null) {
+ this.defaultTopicQueueNums = Integer.parseInt(str);
+ }
+
+ str = fields.get("e");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "queueId");
+ }
+ if (str != null) {
+ this.queueId = Integer.parseInt(str);
+ }
+
+ str = fields.get("f");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "sysFlag");
+ }
+ if (str != null) {
+ this.sysFlag = Integer.parseInt(str);
+ }
+
+ str = fields.get("g");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "bornTimestamp");
+ }
+ if (str != null) {
+ this.bornTimestamp = Long.parseLong(str);
+ }
+
+ str = fields.get("h");
+ if (str == null) {
+ str = getAndCheckNotNull(fields, "flag");
+ }
+ if (str != null) {
+ this.flag = Integer.parseInt(str);
+ }
+
+ str = fields.get("i");
+ if (str == null) {
+ str = fields.get("properties");
+ }
+ if (str != null) {
+ this.properties = str;
+ }
+
+ str = fields.get("j");
+ if (str == null) {
+ str = fields.get("reconsumeTimes");
+ }
+ if (str != null) {
+ this.reconsumeTimes = Integer.parseInt(str);
+ }
+
+ str = fields.get("k");
+ if (str == null) {
+ str = fields.get("unitMode");
+ }
+ if (str != null) {
+ this.unitMode = Boolean.parseBoolean(str);
+ }
+
+ str = fields.get("l");
+ if (str == null) {
+ str = fields.get("maxReconsumeTimes");
+ }
+ if (str != null) {
+ this.maxReconsumeTimes = Integer.parseInt(str);
+ }
+
+ str = fields.get("m");
+ if (str == null) {
+ str = fields.get("batch");
+ }
+ if (str != null) {
+ this.batch = Boolean.parseBoolean(str);
+ }
+
+ str = fields.get("n");
+ if (str == null) {
+ str = fields.get("brokerName");
+ }
+ if (str != null) {
+ this.setBrokerName(str);
+ }
Review Comment:
`decode()` falls back to `fields.get("brokerName")`, but the inherited field name used in extFields for v1 requests is `bname` (from `RpcRequestHeader`). With current code, v1 requests that include broker name under `bname` won't populate the header's broker name.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeader.java:
##########
@@ -183,22 +191,140 @@ public void setBatch(Boolean batch) {
}
public static SendMessageRequestHeader parseRequestHeader(RemotingCommand request) throws RemotingCommandException {
- SendMessageRequestHeaderV2 requestHeaderV2 = null;
- SendMessageRequestHeader requestHeader = null;
- switch (request.getCode()) {
- case RequestCode.SEND_BATCH_MESSAGE:
- case RequestCode.SEND_MESSAGE_V2:
- requestHeaderV2 = request.decodeCommandCustomHeader(SendMessageRequestHeaderV2.class);
- case RequestCode.SEND_MESSAGE:
- if (null == requestHeaderV2) {
- requestHeader = request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
- } else {
- requestHeader = SendMessageRequestHeaderV2.createSendMessageRequestHeaderV1(requestHeaderV2);
- }
- default:
- break;
- }
- return requestHeader;
+ return request.decodeCommandCustomHeader(SendMessageRequestHeader.class);
+ }
+
+ @Override
+ public void encode(ByteBuf out) {
+ writeIfNotNull(out, "a", producerGroup);
+ writeIfNotNull(out, "b", topic);
+ writeIfNotNull(out, "c", defaultTopic);
+ writeIfNotNull(out, "d", defaultTopicQueueNums);
+ writeIfNotNull(out, "e", queueId);
+ writeIfNotNull(out, "f", sysFlag);
+ writeIfNotNull(out, "g", bornTimestamp);
+ writeIfNotNull(out, "h", flag);
+ writeIfNotNull(out, "i", properties);
+ writeIfNotNull(out, "j", reconsumeTimes);
+ writeIfNotNull(out, "k", unitMode);
+ writeIfNotNull(out, "l", maxReconsumeTimes);
+ writeIfNotNull(out, "m", batch);
+ writeIfNotNull(out, "n", getBname());
+ }
Review Comment:
`encode()` is writing short keys ("a".."n") for `RequestCode.SEND_MESSAGE`. This changes the on-wire extFields keys (e.g., topic becomes "b" instead of "topic", brokerName becomes "n" instead of "bname") and will break components that read extFields directly (e.g., authorization builder expects "topic"/"producerGroup", proxy expects "bname") and also breaks backward compatibility with older brokers/clients that decode by field name.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeaderV2.java:
##########
@@ -20,6 +20,7 @@
import com.google.common.base.MoreObjects;
import io.netty.buffer.ByteBuf;
import java.util.HashMap;
+import java.util.Map;
Review Comment:
`java.util.Map` is imported but never used in this class; this can fail style/unused-import checks.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageResponseHeader.java:
##########
@@ -22,6 +22,7 @@
import io.netty.buffer.ByteBuf;
import java.util.HashMap;
+import java.util.HashMap;
Review Comment:
Duplicate `HashMap` import; this can fail style/unused-import checks and adds noise.
##########
remoting/src/main/java/org/apache/rocketmq/remoting/protocol/header/SendMessageRequestHeader.java:
##########
@@ -21,19 +21,23 @@
package org.apache.rocketmq.remoting.protocol.header;
import com.google.common.base.MoreObjects;
+import io.netty.buffer.ByteBuf;
+import java.util.HashMap;
+import java.util.HashMap;
Review Comment:
Duplicate `HashMap` import; this can fail style/unused-import checks and adds noise.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10535:
URL: https://github.com/apache/rocketmq/pull/10535#issuecomment-4760730157
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/10535?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `95 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.12%. Comparing base ([`c270c60`](https://app.codecov.io/gh/apache/rocketmq/commit/c270c60f8592aef32722d40c3d1bb1e3168b3aa1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d6b7a89`](https://app.codecov.io/gh/apache/rocketmq/commit/d6b7a89200e32100f52b33f4a5f7865f86a59bc6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on develop.
| [Files with missing lines](https://app.codecov.io/gh/apache/rocketmq/pull/10535?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ting/protocol/header/SendMessageRequestHeader.java](https://app.codecov.io/gh/apache/rocketmq/pull/10535?src=pr&el=tree&filepath=remoting%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fremoting%2Fprotocol%2Fheader%2FSendMessageRequestHeader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL2hlYWRlci9TZW5kTWVzc2FnZVJlcXVlc3RIZWFkZXIuamF2YQ==) | 0.00% | [89 Missing :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/10535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ing/protocol/header/SendMessageResponseHeader.java](https://app.codecov.io/gh/apache/rocketmq/pull/10535?src=pr&el=tree&filepath=remoting%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fremoting%2Fprotocol%2Fheader%2FSendMessageResponseHeader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL2hlYWRlci9TZW5kTWVzc2FnZVJlc3BvbnNlSGVhZGVyLmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/10535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #10535 +/- ##
=============================================
- Coverage 48.18% 48.12% -0.07%
- Complexity 13407 13409 +2
=============================================
Files 1377 1377
Lines 100838 100925 +87
Branches 13034 13061 +27
=============================================
- Hits 48590 48569 -21
- Misses 46314 46391 +77
- Partials 5934 5965 +31
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/10535?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10533] Implement FastCodesHeader encode/decode for SendMessageRequestHeader/ResponseHeader [rocketmq]
Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on PR #10535:
URL: https://github.com/apache/rocketmq/pull/10535#issuecomment-4976328162
## Review by github-manager-bot
### Summary
Re-review after rebase (commit `1994607f`, 2026-07-14). This PR implements `FastCodesHeader.encode()/decode()` for `SendMessageRequestHeader` to eliminate reflection-based header serialization on the send-message hot path. Single file change (+143/-18).
### Findings
- **[Info] encode() short-key mapping** — The 14 short keys (a–n) correctly match `SendMessageRequestHeaderV2` field names. Verified mapping: a=producerGroup, b=topic, c=defaultTopic, d=defaultTopicQueueNums, e=queueId, f=sysFlag, g=bornTimestamp, h=flag, i=properties, j=reconsumeTimes, k=unitMode, l=maxReconsumeTimes, m=batch, n=brokerName. ✓
- **[Info] decode() dual-key fallback** — Each field first tries the short key, then falls back to the long key via `getAndCheckNotNull()`. This ensures backward compatibility with both V1 (long-key) and V2 (short-key) wire formats. ✓
- **[Info] parseRequestHeader() simplification** — The old switch-case with V2→V1 conversion is replaced by a single `decodeCommandCustomHeader(SendMessageRequestHeader.class)` call. Since `FastCodesHeader.decode()` handles both short and long keys, this correctly replaces both the V1 direct-decode and V2-then-convert paths. The simplification is sound.
- **[Info] bornTimestamp type change** — Field changed from `Long` (boxed) to `long` (primitive), with a null-safe overload `setBornTimestamp(Long)` that defaults to `0L`. This prevents NPE from unboxing and is consistent with the `@CFNotNull` annotation.
- **[Warning] @CFNotNull validation in decode()** — For `@CFNotNull` fields (producerGroup, topic, bornTimestamp, flag), `getAndCheckNotNull()` will throw `RemotingCommandException` if the key is absent. However, for nullable fields like `defaultTopic`, `queueId`, etc., the same `getAndCheckNotNull()` is used. Verify that `getAndCheckNotNull()` correctly returns `null` (rather than throwing) for fields without `@CFNotNull` — otherwise nullable fields would incorrectly throw when absent.
- **[Info] Missing wire-compatibility test** — Previous review suggested adding a unit test that encodes with V2 format and decodes with the new `FastCodesHeader.decode()`. This test is not present in the current diff. Consider adding one to prevent regression.
### Suggestions
1. Verify `getAndCheckNotNull()` behavior for nullable fields (non-`@CFNotNull`).
2. Consider adding a V2→V1 decode compatibility test.
3. The `bornTimestamp` field type change from `Long` to `long` is a binary-incompatible change for any external code calling `getBornTimestamp()`. Confirm this is acceptable for the release.
---
*Automated review by github-manager-bot*
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org