HADOOP-18082.Add debug log when RPC#Reader gets a Call. #3891
Conversation
This file contains 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. |
|
Here are some failed unit tests: It doesn't look like it has much to do with what I'm submitting. |
tomscut
reviewed
Jan 18, 2022
| @@ -3046,6 +3046,8 @@ private void internalQueueCall(Call call, boolean blocking) | |||
| } else { | |||
| callQueue.add(call); | |||
| } | |||
|
|
|||
| LOG.debug("{} has entered the CallQueue and is waiting to be processed.", call); | |||
There was a problem hiding this comment.
If only callId is output, there may be too little useful information. What do you think?
There was a problem hiding this comment.
Thanks @tomscut for the comment and review.
The detailed log here is this:
2022-01-18 16:37:56,653 DEBUG org.apache.hadoop.ipc.Server: Call#75 Retry#0 org.apache.hadoop.hdfs.protocol.ClientProtocol.getFileInfo from xxxx.xxxx.xxxx.xxxx:port has entered the CallQueue and is waiting to be processed. 2022-01-18 16:37:56,654 DEBUG org.apache.hadoop.ipc.Server: Call#76 Retry#0 org.apache.hadoop.hdfs.protocol.ClientProtocol.getFileInfo from xxxx.xxxx.xxxx.xxxx:port has entered the CallQueue and is waiting to be processed.
When the Call is processed by Hanler, the detailed log is as follows:
2022-01-18 16:37:59,725 [5178363759] - WARN [IPC Server handler 126 on 8025:Server@494] - Slow RPC : getFileInfo took 509 milliseconds to process from client Call#76 Retry#0 org.apache.hadoop.hdfs.protocol.ClientProtocol.getFileInfo from xxxx.xxxx.xxxx.xxxx:port
In my opinion, the Calls recorded in these two places should be consistent.
@tomscut , do you have any good suggestions.
There was a problem hiding this comment.
I'm sorry I made a mistake. The output here is not only callId. And I think it is ok.
There was a problem hiding this comment.
The change to add a DEBUG log seems good, but how about this line instead?
LOG.debug("Call has entered the CallQueue and is waiting to be processed. Call details: {}", call);
Usually we tend to read object's string presentation at the end to not mix up with the log statement, e.g RpcCall's toString() has word "from" and perhaps it becomes bit difficult to read the whole statement at the first sight, but if put at the end of the statement, we know that now we are only looking at RpcCall details:
@Override
public String toString() {
return super.toString() + " " + rpcRequest + " from " + connection;
}
tomscut
approved these changes
Jan 19, 2022
| @@ -3046,6 +3046,8 @@ private void internalQueueCall(Call call, boolean blocking) | |||
| } else { | |||
| callQueue.add(call); | |||
| } | |||
|
|
|||
| LOG.debug("{} has entered the CallQueue and is waiting to be processed.", call); | |||
There was a problem hiding this comment.
I'm sorry I made a mistake. The output here is not only callId. And I think it is ok.
|
Thanks @tomscut . |
virajjasani
reviewed
Jan 21, 2022
| @@ -3046,6 +3046,8 @@ private void internalQueueCall(Call call, boolean blocking) | |||
| } else { | |||
| callQueue.add(call); | |||
| } | |||
|
|
|||
| LOG.debug("{} has entered the CallQueue and is waiting to be processed.", call); | |||
There was a problem hiding this comment.
The change to add a DEBUG log seems good, but how about this line instead?
LOG.debug("Call has entered the CallQueue and is waiting to be processed. Call details: {}", call);
Usually we tend to read object's string presentation at the end to not mix up with the log statement, e.g RpcCall's toString() has word "from" and perhaps it becomes bit difficult to read the whole statement at the first sight, but if put at the end of the statement, we know that now we are only looking at RpcCall details:
@Override
public String toString() {
return super.toString() + " " + rpcRequest + " from " + connection;
}
jianghuazhu
force-pushed
the
HADOOP-18082
branch
from
d3cc513
to
05f7c9e
Compare
jianghuazhu
force-pushed
the
HADOOP-18082
branch
from
05f7c9e
to
8554c43
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Latest change looks good. +1(non-binding) |
|
Thanks @virajjasani . |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @ayushtkn . |
|
Hi @ayushtkn , can this pr be merged into trunk branch or other main branch? |
|
Merged. Thanx Everyone!!! |
asfgit
pushed a commit
that referenced
this pull request
Feb 28, 2022
…. Contributed by JiangHua Zhu." Exposes a Race Condition. Which leads to test failures in YARN. (HADOOP-18143) This reverts commit 2025243.
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…. Contributed by JiangHua Zhu.
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Log some information from the moment Call enters the RPC inside, which will help us understand more about Call.
The records here should be in the form of logs. The priority of the logs should not be too high, and debugging is the best.
Details: HADOOP-18082
How was this patch tested?
Corresponding to the test, the pressure is not large.