[jira] [Commented] (HDFS-8449) Add tasks count metrics to datanode for ECWorker

["Rakesh R (JIRA)" <ji...@apache.org>]

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

Rakesh R commented on HDFS-8449:
--------------------------------

Nice work, thanks [~libo-intel] for the patch. I've few comments, please take a look at it.

# Insead of {{recovery}} please use {{reconstruction}} for the newly adding code. HDFS-7955 task has raised to correct the existing code.
# {{"Count of EC recovery tasks"}} here can you describe as {{Erasure Coding}} instead of {{EC}}
# I prefer to avoid {{taskSucceed = false;}}. One way is to keep the {{datanode.getMetrics().incrECSuccessfulRecoveryTasks();}} right before the catch throwable statement.
# Please correct the indentation
a) Make it one line
{code}
+public class
+    TestDataNodeErasureCodingMetrics {
{code}
b) Split this into two lines
{code}
+    out.close();                                           conf = new Configuration();
{code}
# Could you please refer [TestDataNodeMetrics.java#L131|https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeMetrics.java#L131] unit test case. It would be good to use the existing {{MetricsAsserts}} instead of reading the metrics value using relection in your tests.
{code}
+  private long getMetricsValue(String name, DataNodeMetrics metrics)
+      throws Exception{
+    Field ecField = DataNodeMetrics.class.getDeclaredField(name);
+    ecField.setAccessible(true);
+    MutableCounterLong counter = (MutableCounterLong)ecField.get(metrics);
+    return counter.value();
+  }
{code}
# Please make this function to {{private}} visibility
{code}public DataNode doTest{code}
# Its good practice to add test timeout, please add it by giving some bigger value {{@Test(timeout = 120000)}}

> Add tasks count metrics to datanode for ECWorker
> ------------------------------------------------
>
>                 Key: HDFS-8449
>                 URL: https://issues.apache.org/jira/browse/HDFS-8449
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Li Bo
>            Assignee: Li Bo
>         Attachments: HDFS-8449-000.patch, HDFS-8449-001.patch, HDFS-8449-002.patch
>
>
> This sub task try to record ec recovery tasks that a datanode has done, including total tasks, failed tasks and sucessful tasks.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)