[GitHub] [hadoop] functioner commented on pull request #2727: HADOOP-17552. Change ipc.client.rpc-timeout.ms from 0 to 120000 by default to avoid potential hang

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

functioner commented on pull request #2727:
URL: https://github.com/apache/hadoop/pull/2727#issuecomment-789587539


   > @functioner According to CI results, TestIPC#testClientGetTimeout fails. It is related, please check.
   
   It fails at line 1459:
   https://github.com/apache/hadoop/blob/b4985c1ef277bcf51eec981385c56218ac41f09e/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java#L1456-L1460
   
   `Client.getTimeout` is:
   https://github.com/apache/hadoop/blob/b4985c1ef277bcf51eec981385c56218ac41f09e/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L237-L258
   
   Before we change the default rpcTimeout:
   rpcTimeout is 0, so it won't return at line 251.
   `CommonConfigurationKeys.IPC_CLIENT_PING_DEFAULT` is true, so it won't return at line 255 either.
   Finally, it returns -1 at line 257, and passes the test case.
   
   After we change the default rpcTimeout=120000:
   It returns at line 251, it fails because 120000 is not -1.
   
   Conclusion:
   This test is essentially checking the default value of rpcTimeout.
   Since we modified this value, we should also modify this test as `assertThat(Client.getTimeout(config)).isEqualTo(120000)`.
   What do you think? @ferhui @iwasakims 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org