[ 
https://issues.apache.org/jira/browse/HDFS-12772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16334067#comment-16334067
 ] 

Yiqun Lin edited comment on HDFS-12772 at 1/22/18 9:15 AM:
-----------------------------------------------------------

Hi [~elgoiri], just some initial comments for the patch:

*RouterStore .java*
 1. The method name {{getRouterRegistration}} and {{getRouterRegistrations}} is 
so similar, can we rename {{getRouterRegistrations}} to other name?

*GetRouterRegistrationResponse .java*
 Why not also add the field of timestamp in this class like 
\{{GetRouterRegistrationsResponse}} has?

*GetRouterRegistrationRequest .java*
 Can we update the parameter name {{public abstract void setRouterId(String 
router);}} from {{router}} to {{routerId}}?

*RouterHeartbeatRequest .java*
 Can we using the paramter {{routerState}} or {{state}} instead of {{router}}?

*RouterHeartbeatRequestPBImpl .java*
 Please format line {{RouterStatePBImpl routerPB = (RouterStatePBImpl)router;}}.

*RouterState.java*
 Why we use compile date info for setting {{buildVersion}}? Maybe we should add 
a new method (like {{org.apache.hadoop.util.VersionInfo._getBuildVersion()}}) 
in class FederationUtil and use that?

*StateStoreVersion.java*
 I'm not so clear for the usage of this class. Would you document the javadoc 
for this class?

BTW, please fix the compile error in the next patch. Would you mind filling 
other JIRAs for tracking the remaining work?


was (Author: linyiqun):
Hi [~elgoiri], just some initial comments for the patch:

*RouterStore .java*
 1. The method name {{getRouterRegistration}} and {{getRouterRegistrations}} is 
so similar, can we rename {{getRouterRegistrations}} to other name?

*GetRouterRegistrationResponse .java*
 Why not also add the field of timestamp in this class like 
\{{GetRouterRegistrationsResponse }} has?

 *GetRouterRegistrationRequest .java*
 Can we update the parameter name {{public abstract void setRouterId(String 
router);}} from {{router}} to {{routerId}}?

*RouterHeartbeatRequest .java*
 Can we using the paramter {{routerState}} or {{state}} instead of {{router}}?

*RouterHeartbeatRequestPBImpl .java*
 Please format line {{RouterStatePBImpl routerPB = (RouterStatePBImpl)router;}}.

*RouterState.java*
 Why we use compile date info for setting {{buildVersion}}? Maybe we should add 
a new method (like {{org.apache.hadoop.util.VersionInfo._getBuildVersion()}}) 
in class FederationUtil and use that?

*StateStoreVersion.java*
 I'm not so clear for the usage of this class. Would you document the javadoc 
for this class?

BTW, please fix the compile error in the next patch. Would you mind filling 
other JIRAs for tracking the remaining work?

> RBF: Track Router states
> ------------------------
>
>                 Key: HDFS-12772
>                 URL: https://issues.apache.org/jira/browse/HDFS-12772
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>    Affects Versions: 3.0.0
>            Reporter: Íñigo Goiri
>            Assignee: Íñigo Goiri
>            Priority: Major
>         Attachments: HDFS-12772.000.patch, HDFS-12772.001.patch
>
>
> To monitor the state of the cluster, we should track the state of the 
> routers. This should be exposed in the UI.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to