[GitHub] [rocketmq] areyouok commented on pull request #3382: FIX ISSUE#2706 the problem of returning SEND_OK after flush failed,The original pull request was #2707.

[GitBox <gi...@apache.org>]

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


   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?
   ```java
   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.
   
   3:  if the send method returns a SendStatus it means somewhat success, so you should not add FLUSH_DISK_FAILED to SendStatus. 
   
   


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