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

Ayush Saxena commented on HDFS-15159:
-------------------------------------

bq. to make sure whether same DN getting added twice for one block
Well..the test works for me with one too(passes with the fix, fails otherwise), 
but if there is no other way to do, put a comment explaining the two time 
logic, so it doesn't bother someone seeing this in future.
Secondly :

{code:java}
+          if (!excludedNodes.contains(dn.getDatanodeDescriptor())) {
+            excludedNodes.add(dn.getDatanodeDescriptor());
+          }
{code}
{{excludedNodes}} is a Set, No need to check contains and then add. You can 
directly add.

If you want to prevent the loop, you can try streams too, something like this :

{code:java}
      List<DatanodeDescriptor> datanodeDescriptors =
          targets.stream().map(x -> x.getDatanodeDescriptor())
              .collect(Collectors.toList());
      excludedNodes.addAll(datanodeDescriptors);
{code}

I am Ok, with the present way of for loop too, keep whichever way you like. Do 
put up a line of comment too above this, like Exclude all nodes which.....

Other than these nits, fix looks good

> Prevent adding same DN multiple times in PendingReconstructionBlocks
> --------------------------------------------------------------------
>
>                 Key: HDFS-15159
>                 URL: https://issues.apache.org/jira/browse/HDFS-15159
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: hemanthboyina
>            Assignee: hemanthboyina
>            Priority: Major
>         Attachments: HDFS-15159.001.patch, HDFS-15159.002.patch, 
> HDFS-15159.003.patch
>
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to