HADOOP-18885. Add rpcCallsRejectedByObserver metric to quantify the number of rejected RPCs by Observer NameNode #6043
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
Author
|
Hi Sirs @Hexiaoqiao @ayushtkn @goiri @ZanderXu @simbadzina @slfan1989 Could you please help me review this pr when you have free time ? Thanks a lot~ |
|
🎊 +1 overall
This message was automatically generated. |
simbadzina
approved these changes
Sep 21, 2023
Member
simbadzina
left a comment
simbadzina
left a comment
There was a problem hiding this comment.
LGTM. Just one suggestion to make it easier to figure out where the retriable exception is coming from.
| } | ||
| } | ||
| } catch (IOException ioe) { | ||
| if (ioe instanceof RetriableException) { |
Member
There was a problem hiding this comment.
Consider putting a try catch blocked around receiveRequestState. This makes it easier to see where the exception is coming from, instead of checking the type of exception later. So something like
try {
stateId = alignmentContext.receiveRequestState(header, getMaxIdleTime());
} catch (RetriableException re) {
rpcMetrics.incrRcRejectedByObserverCalls();
throw re;
}
Contributor
Author
There was a problem hiding this comment.
Sir @simbadzina thanks help me review and suggestions.
I will update later PR.
…umber of rejected RPCs by Observer NameNode
haiyang1987
force-pushed
the
HADOOP-18885
branch
from
d0c26cb to
22aa648
Compare
Contributor
Author
|
Update PR. |
simbadzina
reviewed
Sep 22, 2023
Member
simbadzina
left a comment
simbadzina
left a comment
There was a problem hiding this comment.
When you push new commits please just push without using force push so that the pull request's history is preserved.
| /** | ||
| * Increments the Observer NameNode rejected RPC Calls Counter. | ||
| */ | ||
| public void incrRcRejectedByObserverCalls() { |
Member
There was a problem hiding this comment.
Can you expand Rc to Rpc. You can rename this to incrRpcCallsRejectedByObserver.
simbadzina
reviewed
Sep 22, 2023
| @Metric("Number of successful RPC calls") | ||
| MutableCounterLong rpcCallSuccesses; | ||
| @Metric("Number of observer namenode rejected RPC calls") | ||
| MutableCounterLong rpcRejectedByObserverCalls; |
Member
There was a problem hiding this comment.
I suggest renaming to rpcCallsRejectedByObserver
Contributor
Author
There was a problem hiding this comment.
Thanks sir suggestions, i will update it.
haiyang1987
changed the title
Contributor
Author
|
Hi @simbadzina sir please help me review this pr again when you have free time. Thanks a lot~ |
simbadzina
approved these changes
Sep 22, 2023
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Member
|
Build failure looks unrelated to your changes. Try pushing an empty commit in order to retrigger Yetus. |
Contributor
Author
yeah，i will push an empty commit in order to retrigger Yetus again |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
The failed unit test seems unrelated to the change. |
goiri
approved these changes
Sep 25, 2023
Contributor
Author
|
@goiri Hi Sir, Could you please help me push this modification forward when you have free time ? Thanks a lot~ |
Contributor
Author
|
Hi @goiri @simbadzina sirs , can you help me push this modification forward when you have free time ? Thank you very much. |
Contributor
Author
|
Hi @goiri @simbadzina excuse me, could you mind to push this modification forward when you have free time ? Thank you very much. |
ZanderXu
reviewed
Oct 16, 2023
| @Metric("Number of successful RPC calls") | ||
| MutableCounterLong rpcCallSuccesses; | ||
| @Metric("Number of observer namenode rejected RPC calls") | ||
| MutableCounterLong rpcCallsRejectedByObserver; |
Contributor
There was a problem hiding this comment.
Thanks @haiyang1987 for your report. And thanks @simbadzina @goiri for your review.
I'm just concern if it's reasonable that rpcCallsRejectedByObserver is in RpcMetrics.java. I think rpcCallsRejectedByObserver is a logic of namenode, so how about changing this logic as a HDFS issue, not common issue?
Contributor
Author
There was a problem hiding this comment.
Thanks @ZanderXu for your comment.
@goiri @simbadzina What do you think? If necessary, I will modify it into an HDFS issue to achieve this requirement.
thanks~
Contributor
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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
https://issues.apache.org/jira/browse/HADOOP-18885
When observer Node's serverStateId is too far behind clientStateId will rejected client RPC request,
mabe we need add rpcRejectedByObserverCalls metric to RpcMetrics to quantify the number of rejected RPCs by Observer NameNode to help determine how well the server.