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

Ayush Saxena commented on HDFS-14167:
-------------------------------------

Thanx [~elgoiri] for taking this up.The overall patch looks good.

Just not sure about this change::
{code:java}
  public int getNumStaleDataNodes() {
-    return -1;
+    try {
+      return getFederationMetrics().getNumStaleNodes();
+    } catch (IOException e) {
+      LOG.debug("Failed to get number of stale nodes", e.getMessage());
+    }
+    return 0;
   }
{code}

What case we are trying to cover with the exception part.When are we expecting 
IOException?
Why other similar methods aren't checking for it? Like 
getNumDecomLiveDataNodes() or getNumDecomDeadDataNodes() and other similar ones.

Moreover if we are getting the exception and unable to fetch the 
value.Shouldn't we return -1 instead of 0(I guess Zero is a legal value for 
staleDataNodes).It could create wrong perspective.

Other than this.Not part of your change ::

I see in the same class at couple of places this being used::

{code:java}
 return this.router.getMetrics().
{code}

Can we replace it with ::


{code:java}
 getFederationMetrics().
{code}

This would enrich readability and even we the methods can be little more 
consistent with each other.
 

> RBF: Add stale nodes to federation metrics
> ------------------------------------------
>
>                 Key: HDFS-14167
>                 URL: https://issues.apache.org/jira/browse/HDFS-14167
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Íñigo Goiri
>            Assignee: Íñigo Goiri
>            Priority: Major
>         Attachments: HDFS-14167-HDFS-13891.000.patch
>
>
> The federation metrics mimic the Namenode FSNamesystemState. However, the 
> stale datanodes are not collected.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to