-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16313. Add metrics for each subcluster #3638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
*/ | ||
@InterfaceAudience.Private | ||
@InterfaceStability.Evolving | ||
public interface SubClusterRPCMBean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we are consistent with terminology.
Sometimes we use subcluster at a high level but then throughout the code we refer to it as namespace.
Can you check what is the most consistent terminology throughout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Aligned to use "subcluster".
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@goiri I got the idea to use "subcluster" because of https://hadoop.apache.org/docs/stable/hadoop-project-dist/hadoop-hdfs-rbf/HDFSRouterFederation.html. I chose subcluster earlier because I thought compared to "FederationRPCMetrics", it might be easier for users to know that we are talking about the metrics of the underlayer of RBF to use "SubclusterRPCMetrics" than "NameserviceRPCMetrics". Both names would be fine to me, I'll make the change after there is an alignment. |
💔 -1 overall
This message was automatically generated. |
|
||
@Override | ||
public void proxyOpNoNamenodes() { | ||
public void proxyOpNoNamenodes(String nameservice) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nsId might be a little better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
private MutableCounterLong proxyOpNoNamenodes; | ||
|
||
public NameserviceRPCMetrics(Configuration conf, String name) { | ||
this.name = name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nsId?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
* @param success If the operation was successful. | ||
*/ | ||
void proxyOpComplete(boolean success); | ||
void proxyOpComplete(boolean success, String ns); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nsId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
💔 -1 overall
This message was automatically generated. |
@goiri Do you have any other comments? |
Description of PR
Currently we have metrics to track the operations for Router to all nameservices, like "FederationRPCMetrics", but we don't have metrics for Router to each nameservices.
This ticket is to add metrics for each nameservice to better track the performance of each sub cluster.
Jira ticket: https://issues.apache.org/jira/browse/HDFS-16313
How was this patch tested?
unit test
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?