iloveeyjafjalla opened a new pull request, #11426:
URL: https://github.com/apache/rocketmq/pull/11426
### Which Issue(s) This PR Fixes
- Fixes #11425
### Brief Description
`BrokerOuterAPI.registerBrokerAll` compressed the register body but
hardcoded the header flag that tells the name server whether the body is
compressed:
```java
requestHeader.setCompressed(false); // :517
...
final byte[] body = requestBody.encode(compressed); // :525 — deflate
when compressed == true
```
The name server decodes strictly by that flag
(`RegisterBrokerBody.decode(request.getBody(), requestHeader.isCompressed(),
brokerVersion)` in `DefaultRequestProcessor`, and `if (!compressed) { return
decode(data, RegisterBrokerBody.class); }` in `RegisterBrokerBody`), so with
`compressedRegister=true` it tried to parse a deflate stream as UTF-8 JSON:
every register RPC failed, `registerBrokerAll` returned an empty result list,
the failures were only logged, and the broker never appeared in any route table.
The flag is wired from configuration (`BrokerController.doRegisterBrokerAll`
→ `brokerConfig.isCompressedRegister()`, used at startup and periodically), and
this is a regression from RIP-32: commit `cad0cfcec` changed
`requestHeader.setCompressed(compressed)` into `setCompressed(false)` while
keeping `encode(compressed)`.
The change restores the line:
```java
requestHeader.setCompressed(compressed);
```
so body and flag describe the same thing — exactly what
`RegisterBrokerBodyTest` already requires (it encodes with `encode(true)` and
decodes with `decode(..., true, ...)`).
### How Did You Test This Change?
New test
`BrokerOuterAPITest#test_register_compressed_body_matches_header_flag`: it
registers with `compressed = true` (the harness already used that value but
never inspected the request), captures the `RemotingCommand` handed to
`nettyRemotingClient.invokeSync`, asserts the captured header flags the body as
compressed, and decodes the captured body with **that same flag** — the way the
name server does.
Results on Temurin JDK 8 (the JDK CI uses), base `a7edeecfb` plus this patch:
- before the fix: `java.lang.AssertionError: a compressed register body has
to be flagged as compressed`
- after the fix: `BrokerOuterAPITest` — `Tests run: 12, Failures: 0, Errors:
0, Skipped: 0`
- `mvn -B -ntp -pl broker clean package`: `You have 0 Checkstyle
violations.` and `BugInstance size is 0`; the test phase reached 99 test
classes with 0 failures / 0 errors and then stalled in
`TransactionalMessageServiceImplTest` on this machine (no further log output
for more than 20 minutes while the forked JVM kept spinning, so the run was
stopped). This is an environment behaviour I also hit in the `store` module
today (a native RocksDB crash after ~45 classes in one JVM) — every affected
class passes on its own. I therefore report the targeted class result plus the
static checks rather than a full-suite claim.
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above were reproduced locally by running
the module's tests against the base commit and the patched tree.
--
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]
