[
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)
