You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/06/29 15:20:37 UTC

[GitHub] [rocketmq] Justinwins opened a new issue, #4533: NettyEventExecutor .putNettyEvent is not thread safe

Justinwins opened a new issue, #4533:
URL: https://github.com/apache/rocketmq/issues/4533

   ```
    class NettyEventExecutor extends ServiceThread {
           private final LinkedBlockingQueue<NettyEvent> eventQueue = new LinkedBlockingQueue<NettyEvent>();
           private final int maxSize = 10000;
   
           public void putNettyEvent(final NettyEvent event) {
               int currentSize = this.eventQueue.size();
               if (currentSize <= maxSize) {
                   this.eventQueue.add(event);   // it's  not thread safe here . 
               } else {
                   log.warn("event queue size [{}] over the limit [{}], so drop this event {}", currentSize, maxSize, event.toString());
               }
           }
   
   ```


-- 
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: dev-unsubscribe@rocketmq.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq] Oliverwqcwrw commented on issue #4533: NettyEventExecutor .putNettyEvent is not thread safe

Posted by GitBox <gi...@apache.org>.
Oliverwqcwrw commented on issue #4533:
URL: https://github.com/apache/rocketmq/issues/4533#issuecomment-1170674417

   IMO, although it is thread unsafe, it will only have a few more events, and it does not seem to have a serious impact, WDYT?


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq] lizhiboo commented on issue #4533: NettyEventExecutor .putNettyEvent is not thread safe

Posted by GitBox <gi...@apache.org>.
lizhiboo commented on issue #4533:
URL: https://github.com/apache/rocketmq/issues/4533#issuecomment-1170652551

   @Justinwins IllegalStateException("Queue full") will not be thrown if using offer method, and then, channel will not be closed although NettyEvent is lost. If the lost event is NettyEventType.CLOSE, still using this channel will make mistake.


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq] lizhanhui commented on issue #4533: NettyEventExecutor .putNettyEvent is not thread safe

Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #4533:
URL: https://github.com/apache/rocketmq/issues/4533#issuecomment-1171878588

   @Oliverwqcwrw The crux here is can we afford dropping events in case there are too many netty events... From design perspective, we should not drop events here, otherwise, we have to compensate logic by way to schedule scanning wherever relies on channel state...
   This is why ClientHouseKeepingService has a scheduled thread: https://github.com/apache/rocketmq/blob/a62b70bc25423c1d7e18043e32af427d29ef9ac4/broker/src/main/java/org/apache/rocketmq/broker/client/ClientHousekeepingService.java#L43
   
   Obviously, it is a trade-off...when there are a massive number of events to handle and the ChannelEventListener implementation cannot process them timely...drop those events and let the schedule scanning to maintain eventual consistency. 
   
   To be honest, it's pretty error-prone when implementing new features that may rely on channel state. They are very likely to forget to implement the compensation logic. 
   
   A better alternative is to ensure every netty event is handled.


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq] lizhanhui closed issue #4533: NettyEventExecutor .putNettyEvent is not thread safe

Posted by GitBox <gi...@apache.org>.
lizhanhui closed issue #4533: NettyEventExecutor .putNettyEvent is not thread safe
URL: https://github.com/apache/rocketmq/issues/4533


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [rocketmq] lizhanhui commented on issue #4533: NettyEventExecutor .putNettyEvent is not thread safe

Posted by GitBox <gi...@apache.org>.
lizhanhui commented on issue #4533:
URL: https://github.com/apache/rocketmq/issues/4533#issuecomment-1171879151

   @lizhiboo 


-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org