functioner commented on a change in pull request #2878:
URL: https://github.com/apache/hadoop/pull/2878#discussion_r613361072
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogAsync.java
##########
@@ -378,13 +381,18 @@ public void logSyncWait() {
@Override
public void logSyncNotify(RuntimeException syncEx) {
- try {
- if (syncEx == null) {
- call.sendResponse();
- } else {
- call.abortResponse(syncEx);
+ for (int retries = 0; retries <= RESPONSE_SEND_RETRIES; retries++) {
Review comment:
@daryn-sharp, in our report
([HDFS-15957](https://issues.apache.org/jira/browse/HDFS-15957)), we are doing
fault injection testing, and we inject an IOException in `call.sendResponse()`,
and then we observe the symptom that the client gets stuck. In this scenario,
retry can help.
> If the response cannot be sent it's either because the connection is
already closed or there's a bug preventing the encoding of the response.
Within `call.sendResponse()`, there are lots of code; some of them is
related to the OS kernel I/O, and some of them may get changed in the future,
so I think (now & future) there should be multiple reasons for an IOException,
besides the two you list here. Furthermore, in the scenarios you describe,
retry wouldn't make it worst.
Therefore, I think it's still worth adding the retry logic here, although it
might not be able to handle the two scenarios you describe here. Do you agree?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
