[
https://issues.apache.org/jira/browse/HDFS-8950?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14715631#comment-14715631
]
Colin Patrick McCabe commented on HDFS-8950:
--------------------------------------------
Looks good overall.
{code}
1288 (includedNodes.isEmpty() || includedNodes.match(addr))) {
{code}
Should be {{includedNodes.isIncluded}}, which already has the logic to treat
empty include lists as "everything is included"
{code}
427 Assert.assertThat("Incorrect number of hosts reported",
428 both.size(), is(2));
{code}
This could just be assertEquals(2, ...), which seems better since it would tell
you "this is 3 instead of 2" or something like that rather than just telling
you that it wasn't what was expected.
{code}
450
System.out.println(dm.getDatanodeListForReport(HdfsConstants.DatanodeReportType.ALL));
{code}
Should use the LOG variable in this test class
{code}
405 // Set the includes and excludes lists the hard way because the
406 // HostFileManager wasn't designed for TDD. We have to also set
407 // excludes because it would otherwise be null, causing an NPE.
408 // (The refresh() method would normally fix that, but we don't call
it.)
409 Whitebox.setInternalState(hm, "includes", twoNodes);
410 Whitebox.setInternalState(hm, "excludes", noNodes);
411 Whitebox.setInternalState(dm, "hostFileManager", hm);
{code}
I disagree that the "HostFileManager wasn't designed for TDD." It had unit
tests ever since it was added to the source tree. It was designed to use
immutable state as much as possible in order to avoid the many locking problems
that we previously had with mutable host file state inside the
{{DatanodeManager}}.
For this particular unit test, we don't need complex Mockito stuff. Why not
just add a new {{HostFileManager#refresh}} method annotated with
\@VisibleForTesting that just takes the new includes and excludes that the unit
test wants to set?
thanks
> NameNode refresh doesn't remove DataNodes
> -----------------------------------------
>
> Key: HDFS-8950
> URL: https://issues.apache.org/jira/browse/HDFS-8950
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: datanode, namenode
> Affects Versions: 2.6.0
> Reporter: Daniel Templeton
> Assignee: Daniel Templeton
> Fix For: 2.8.0
>
> Attachments: HDFS-8950.001.patch, HDFS-8950.002.patch,
> HDFS-8950.003.patch
>
>
> If you remove a DN from NN's allowed host list (HDFS was HA) and then do NN
> refresh, it doesn't remove it actually and the NN UI keeps showing that node.
> It may try to allocate some blocks to that DN as well during an MR job. This
> issue is independent from DN decommission.
> To reproduce:
> 1. Add a DN to dfs_hosts_allow
> 2. Refresh NN
> 3. Start DN. Now NN starts seeing DN.
> 4. Stop DN
> 5. Remove DN from dfs_hosts_allow
> 6. Refresh NN -> NN is still reporting DN as being used by HDFS.
> This is different from decom because there DN is added to exclude list in
> addition to being removed from allowed list, and in that case everything
> works correctly.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
