iamqq23ue commented on pull request #3382:
URL: https://github.com/apache/rocketmq/pull/3382#issuecomment-975536773


   I don't agree with the current commit.
    
   1: In SYNC_FLUSH mode it use MappedByteBuffer.force() method. The javadoc of 
this method is ambiguous and there is no exception throws. Do you get the 
exception of this line?
    log.error("Error occurred when force data to disk.", e);
    
   I guess there is no exception if IO operation fails (maybe just return with 
no guarantee, or hang).
   
   ------------------------------------------------
   我测试过了,日志里面可以看到报错。
   
   ------------------------------------------------
    
   2: Even if unrecoverable exceptions throws in public int flush(final int 
flushLeastPages) method (maybe cause by FileChannel.force), we should mark 
broker as read only mode(broker permission 4, will update name server and no 
more requests send to the broker), but not fails in each send operation.
   
   ---------------------------------------------
   
   
您说的解决方法很好。但是目前RocketMQ报错码的处理,都是通过反馈客户端,让客户端进行处理,没有看到哪个报错码会直接影响broker的。因此我修改代码,尽量和之前的代码理念保持一致。如果社区觉得可以直接通过exception处理将broker设置为只读,我没有任何意见。
   
   ---------------------------------------------
    
   3:  if the send method returns a SendStatus it means somewhat success, so 
you should not add FLUSH_DISK_FAILED to SendStatus.
    
   —----------------------------
   
   有一类SendStatus会触发retry的,并不是我首创,我只是加了一个报错返回而已
   
   -----------------------------
   You are receiving this because you authored the thread.
   Reply to this email directly, view it on GitHub, or unsubscribe.
   Triage notifications on the go with GitHub Mobile for iOS or Android.


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