fuyou001 commented on PR #10910:
URL: https://github.com/apache/rocketmq/pull/10910#issuecomment-5277266821

   **[High] Coalesce the wake-up sentinel to preserve notification and 
bounded-queue semantics**
   
   ServiceThread.wakeup() intentionally coalesces repeated notifications 
through hasNotified, but this override unconditionally executes 
eventQueue.offer(wakeupEvent), even when the parent wakeup has already become a 
no-op. Because eventQueue is an unbounded LinkedBlockingQueue and 
putNettyEvent() starts dropping normal events once the queue exceeds 10,000 
entries, repeated or concurrent wakeup() calls, inherited makeStop() calls, or 
restart races can retain an unbounded number of sentinel nodes and eventually 
cause real CLOSE, IDLE, or EXCEPTION events to be dropped. That can skip 
broker, client, route, or heartbeat cleanup.
   
   I reproduced this on the current head: 10,001 wakeup() calls resulted in 
10,001 queued sentinel entries, whereas the parent notification contract 
coalesces them.
   
   Please guard the control marker with an atomic pending state so at most one 
sentinel can be queued, and reset that state safely when the marker is 
consumed, including shutdown-before-run and restart races. A regression test 
should block the listener, issue more than 10,000 serial and concurrent 
wakeup()/makeStop() calls, enqueue a real CLOSE event, release the listener, 
and assert that the CLOSE event is delivered exactly once in FIFO order while 
the number of control markers remains bounded. Please also cover 
shutdown(false), shutdown(true), repeated stop, and stop/restart.


-- 
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]

Reply via email to