CuriousLinYu commented on code in PR #8205:
URL: https://github.com/apache/incubator-seata/pull/8205#discussion_r3900604140
##########
core/src/main/java/org/apache/seata/core/rpc/netty/AbstractNettyRemotingClient.java:
##########
@@ -587,6 +587,20 @@ public void run() {
while (true) {
mergeLock.lock();
try {
+ // Park until there are pending messages, so the merge
thread no longer
+ // burns CPU with a 1ms polling cycle when idle. The
check-and-wait is
+ // atomic under mergeLock and producers offer to the
basket before
+ // signalling (see sendSyncRequest), so no wake-up can be
lost.
+ while (isBasketEmpty()) {
+ isSending = false;
+ mergeCondition.await();
Review Comment:
Thanks for the review — both points were spot on. I've removed the
post-wakeup `await(1ms)` entirely: the thread now drains immediately after
being signalled. Measured locally: first-message drain latency drops from
~1.4-15.3ms (with the 1ms await) to ~0.09-0.21ms, and batching does not regress
— avg batch size is 50 vs 21.3 on the original under a 4x25 concurrent burst.
The full measurement table is in the PR description.
On the 30% CPU number — fair point that Arthas sampling and container CPU
quota matter. What the issue confirms is the thread dump: the
`rpcMergeMessageSend_*_1` threads sit in `TIMED_WAITING` while burning CPU.
Locally (ThreadMXBean, 10s window) the original implementation consumes a
steady 31ms of thread CPU time per 10s at idle; this PR takes it to 0. The
absolute ratio is environment-dependent, but the idle wake-up cost is real and
now gone.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
