HADOOP-18324. Interrupting RPC Client calls can lead to thread exhaustion. #4527
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@omalley I'm very happy that you noticing this issue and working on it. This problem occurs frequently in out prod environment when the network is abnormal.
In addition to guaranteeing that only one request is sent at the same time, does it need to guarantee the order of sending? |
Contributor
Author
|
The extra thread is used so that if the calling thread is interrupted while the request is being written to the socket, that the write is not interrupted. Basically, it is ensuring that the socket doesn't have partial requests sent to it. Without that, Hadoop would need to resynchronize by closing the socket. |
Contributor
Author
|
My patch also guarantees that the requests are written in the order they arrived, which the current code does not. |
Contributor
Nice explain, I learned a log. Thanks very much. |
hotcodemacha
approved these changes
Jul 4, 2022
Contributor
|
+1 (non-binding). lets make the checkstyle happy as well |
Contributor
Author
|
@steveloughran Can you recommend someone to review this RPC patch? |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
Ask on the HDEFS mailing list; they are the ones that get unhappy first when things break. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Outdated
Show resolved
Hide resolved
mccormickt12
approved these changes
Jul 13, 2022
|
🎊 +1 overall
This message was automatically generated. |
Hexiaoqiao
reviewed
Jul 15, 2022
Contributor
Hexiaoqiao
left a comment
Hexiaoqiao
left a comment
There was a problem hiding this comment.
@omalley Thanks for your works. Leave one question I am very clear. Others look good to me.
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Show resolved
Hide resolved
ZanderXu
reviewed
Aug 1, 2022
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Show resolved
Hide resolved
ZanderXu
requested changes
Aug 18, 2022
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
Outdated
Show resolved
Hide resolved
omalley
force-pushed
the
hadoop-18324
branch
from
38169e5 to
4df3227
Compare
Contributor
Author
|
Sorry, I got busy with other work. I've included fixes that we've deployed for a while now. I had missed some cases of IOException handling and start up issues. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
omalley
added a commit
that referenced
this pull request
Nov 18, 2022
…tion. (#4527) * Exactly 1 sending thread per an RPC connection. * If the calling thread is interrupted before the socket write, it will be skipped instead of sending it anyways. * If the calling thread is interrupted during the socket write, the write will finish. * RPC requests will be written to the socket in the order received. * Sending thread is only started by the receiving thread. * The sending thread periodically checks the shouldCloseConnection flag.
omalley
added a commit
that referenced
this pull request
Nov 18, 2022
…tion. (#4527) * Exactly 1 sending thread per an RPC connection. * If the calling thread is interrupted before the socket write, it will be skipped instead of sending it anyways. * If the calling thread is interrupted during the socket write, the write will finish. * RPC requests will be written to the socket in the order received. * Sending thread is only started by the receiving thread. * The sending thread periodically checks the shouldCloseConnection flag.
omalley
added a commit
that referenced
this pull request
Nov 18, 2022
…tion. (#4527) * Exactly 1 sending thread per an RPC connection. * If the calling thread is interrupted before the socket write, it will be skipped instead of sending it anyways. * If the calling thread is interrupted during the socket write, the write will finish. * RPC requests will be written to the socket in the order received. * Sending thread is only started by the receiving thread. * The sending thread periodically checks the shouldCloseConnection flag.
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…tion. (apache#4527) * Exactly 1 sending thread per an RPC connection. * If the calling thread is interrupted before the socket write, it will be skipped instead of sending it anyways. * If the calling thread is interrupted during the socket write, the write will finish. * RPC requests will be written to the socket in the order received. * Sending thread is only started by the receiving thread. * The sending thread periodically checks the shouldCloseConnection flag.
steveloughran
added a commit
to steveloughran/hadoop
that referenced
this pull request
Feb 8, 2023
…d exhaustion. (apache#4527)" Pulling out of this release because of the problems of HDFS-16853 This reverts commit bc4d7b4.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
I modified the RPC Connection class to use a single thread that does the writes to the socket instead of a thread factory. (It already has a different thread that does the reads.) Java’s concurrency library has a SynchronousQueue that will simplify hand offs from the calling thread to the rpc sending thread.
As a result, we’ll end up with:
How was this patch tested?
Added a unit test.