wang-jiahua opened a new issue, #10898: URL: https://github.com/apache/rocketmq/issues/10898
### Before Creating the Enhancement Request - [x] I have confirmed that this should be classified as an enhancement rather than a bug/feature. ### Summary `NettyEventExecutor` blocks in `eventQueue.poll(3000, TimeUnit.MILLISECONDS)`, but `ServiceThread.shutdown()` signals a stop through the `stopped` flag plus `wakeup()`. `wakeup()` only unparks a thread waiting inside `waitForRunning`; it cannot release a thread blocked on a `LinkedBlockingQueue`. The thread therefore keeps waiting until the poll expires before it observes the stopped flag, so every shutdown of a remoting instance waits up to 3 seconds for nothing. A single `BrokerController.shutdown()` owns several remoting instances, so the cost adds up. Local run of `BrokerShutdownTest` on `develop`: ``` join thread[NettyEventExecutor], elapsed time: 2993ms, join time:90000ms join thread[NettyEventExecutor], elapsed time: 2999ms, join time:90000ms join thread[NettyEventExecutor], elapsed time: 2999ms, join time:90000ms ``` Three instances, roughly 9 seconds of pure waiting per broker shutdown. ### Motivation Shutdown latency is paid in production on every graceful restart, and in CI on every test that starts and stops a broker or client. It also contributes to #10823: `BrokerShutdownTest` performs four full start/stop cycles, and this waiting is a large part of why the class exceeds the Bazel small test timeout. ### Describe the Solution You'd Like Override `wakeup()` in `NettyEventExecutor` to offer a sentinel event to `eventQueue` after delegating to `super.wakeup()`, so the pending `poll` returns immediately and the loop re-checks the stopped flag. The dispatch loop skips the sentinel by reference, so no listener callback is triggered and `NettyEventType` does not need a new constant. Measured locally with this change: | | before | after | |---|---|---| | `join thread[NettyEventExecutor]` | ~3000 ms each | 0 ms each | | `BrokerShutdownTest` | 71.98 s | 53.63 s | | `NettyRemotingAbstractTest` | 3.76 s | 1.08 s | `remoting` is green at 175/175 with zero Checkstyle violations. ### Describe Alternatives You've Considered Shortening the poll timeout, for example to 500 ms. That reduces the worst-case wait but does not remove it, and it multiplies idle wakeups for a queue that is empty most of the time. Interrupting the thread on shutdown. `ServiceThread` deliberately keeps `shutdown()` free of interruption and offers `shutdown(true)` for callers that want it, so relying on interruption here would work against that design. ### Additional Context The sentinel is a private field compared by reference, so it cannot collide with a real event, and `putNettyEvent` is left untouched to keep the existing queue-size guard for genuine channel events. -- 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]
