wang-jiahua commented on PR #10640: URL: https://github.com/apache/rocketmq/pull/10640#issuecomment-5042966962
Thanks for the thorough review, @fuyou001. I've addressed all three points. ## 1. Unit tests Added `CheckMessageBufferReuseTest` (store module) covering the four cases you listed: - **reuse** across multiple messages on the same thread — the same buffer instance is returned; - **grow-only** behavior when a larger message arrives, and the grown buffer is then reused for subsequent smaller messages; - the **transient path** when the requested size exceeds the reuse cap — and it verifies the oversized buffer is **not retained** in the `ThreadLocal` (a later small request still returns the previously cached small buffer); - **corrupt `totalSize`** (negative, and larger-than-remaining) is rejected with `DispatchRequest(-1, false)` and no exception. `AppendPropCRCTest` (the existing coverage of `checkMessageAndReturnSize`) still passes. Together: 6 tests, 0 failures; 0 checkstyle violations. ## 2. Configurable upper bound for the reusable buffer (agreed) I decoupled the reuse cap from `maxMessageSize` by introducing a dedicated, configurable bound: - New `MessageStoreConfig.maxCheckMessageReuseBufferSize`, default **4 MiB + 64 KiB** (identical to the previous cap under the default `maxMessageSize`, so default behavior is unchanged). - `borrowCheckMessageBuffer` now bypasses reuse and returns a **transient** buffer when `totalSize > maxCheckMessageReuseBufferSize`. So raising `maxMessageSize` (e.g. to 100 MiB) no longer inflates the retained per-thread buffer; operators who want a larger reusable buffer can raise this knob explicitly. On the worst-case retained memory (`reuse threshold × thread count`): `checkMessageAndReturnSize` is only invoked by **store background threads** — the single `ReputMessageService` dispatch thread, single-threaded startup recovery (`recoverNormally`/`recoverAbnormally`), the HA `AutoSwitchHAService` thread, and the dledger path — **not** per connection/producer. So the participating thread count is small and bounded (O(store background threads)); with the new default, retained memory is bounded at ~`4 MiB + 64 KiB` per such thread. ## 3. Benchmarks **Micro-benchmark** — allocation isolated to the changed buffer (`com.sun.management.ThreadMXBean#getThreadAllocatedBytes`, warmed to steady state), several size distributions, default cap `REUSE_CAP = 4 MiB + 64 KiB`: | distribution | old B/op | new B/op | saved | |---|---:|---:|---:| | uniform 2 KB | 2,320 | 0 | 100% | | mixed small (512 B / 2 KB / 8 KB) | 3,728 | 0 | 100% | | **99% 2 KB + 1% 5 MB (> cap)** | 54,726 | 52,429 | 4.2% | | uniform 1 MB (≤ cap) | 1,048,592 | 0 | 100% | The oversized-message workload confirms the fallback behaves as intended: the rare 5 MB messages allocate a transient buffer (≈ the 1% × 5 MB residual, identical to baseline) and are not retained, while the ≤ cap 99% are served entirely from the reused buffer — i.e. **no regression**, and the benefit scales with the ≤ cap fraction. **End-to-end on real hardware** — 4-machine isolated (Producer 16c / Broker 8c / Consumer 8c / NameServer), same 5.5.0-tag build, per-arm `pkill + rm store + sync + drop_caches`, 3 interleaved rounds, 120 s each, 32 producer threads, 2 KB sync send; Temurin-21, `-Xms4g -Xmx4g`, G1GC: | arm | TPS (median) | P99 ms (median) | young GC / M msgs (median) | |---|---:|---:|---:| | baseline | 98,943 | 0.86 | 9.176 | | this PR | 100,476 | 0.85 | 8.211 | - **Allocation / GC:** young GC per million messages **−10.5%** (three rounds 8.19–8.41 vs baseline 9.07–9.27, clearly separated). - **Throughput:** median TPS ≥ baseline (no regression; per-run ±4% is run noise). - **Latency:** P99 flat at 0.85–0.86 ms. Methodology note: `young GC/msg` (`grep -c 'Pause Young' gc.log` ÷ `msgPutTotalTodayNow` delta) is the primary allocation-pressure metric because it is immune to TPS jitter; exact per-op bytes come from the ThreadMXBean micro-benchmark above. -- 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]
