[jira] [Comment Edited] (HDFS-11085) Add unit tests for NameNode failing to startup when name dir can not be written
[ https://issues.apache.org/jira/browse/HDFS-11085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15634134#comment-15634134 ] Mingliang Liu edited comment on HDFS-11085 at 11/3/16 8:39 PM: --- The patch looks good to me overall. Some minor comments: # Checkstyle warning is related. # MiniDFSCluster can be used in try-with-resource. Shorter and neater. # I think we can use {{Paths.get()}} for joining path strings. Assuming file path separate is / is not platform independent probably. Also if there are methods in JDK or Apache component that work just fine, I hesitate to use third party libraries. {code} 661 final String nnDirStr = Joiner.on("/").join( 662 hdfsDir.toString(), 663 GenericTestUtils.getMethodName(), "name"); {code} # I don't think we need the implementation details about the Collection type (List). We just need the only element in the collections, don't we? {code} 673 /* get and verify NN dir */ 674 List nameDirs = (List) FSNamesystem.getNamespaceDirs(config); {code} Alternatively: {code} final Collection nameDirs = FSNamesystem.getNamespaceDirs(config); assertNotNull(nameDirs); assertTrue(nameDirs.iterator().hasNext()); final URI nameDir = nameDirs.iterator().next(); ... {code} was (Author: liuml07): The patch looks good to me overall. Some minor comments: # Checkstyle warning is related. # MiniDFSCluster can be used in try-with-resource. Shorter and neater. # I think we can use {{Paths.get()}} for joining path strings. Assuming file path separate is / is not platform independent probably. Also if there are methods in JDK or Apache component that work just fine, I hesitate to use third party libraries. {code} 661 final String nnDirStr = Joiner.on("/").join( 662 hdfsDir.toString(), 663 GenericTestUtils.getMethodName(), "name"); {code} # I don't think we need the implementation details about the Collection type (List). We just need the only element in the collections, don't we? {code} 673 /* get and verify NN dir */ 674 List nameDirs = (List) FSNamesystem.getNamespaceDirs(config); {code} Alternatively: {code} Collection nameDirs = FSNamesystem.getNamespaceDirs(config); assertNotNull(nameDirs); assertTrue(nameDirs.iterator().hasNext()); final URI nameDir = nameDirs.iterator().next(); ... {code} > Add unit tests for NameNode failing to startup when name dir can not be > written > --- > > Key: HDFS-11085 > URL: https://issues.apache.org/jira/browse/HDFS-11085 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: namenode, test >Reporter: Mingliang Liu >Assignee: Xiaobing Zhou > Attachments: HDFS-11085.000.patch > > > This can be placed in {{org.apache.hadoop.hdfs.server.namenode.TestStartup}} > test class. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-11085) Add unit tests for NameNode failing to startup when name dir can not be written
[ https://issues.apache.org/jira/browse/HDFS-11085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15634134#comment-15634134 ] Mingliang Liu edited comment on HDFS-11085 at 11/3/16 8:27 PM: --- The patch looks good to me overall. Some minor comments: # Checkstyle warning is related. # MiniDFSCluster can be used in try-with-resource. Shorter and neater. # I think we can use {{Paths.get()}} for joining path strings. Assuming file path separate is / is not platform independent probably. Also if there are methods in JDK or Apache component that work just fine, I hesitate to use third party libraries. {code} 661 final String nnDirStr = Joiner.on("/").join( 662 hdfsDir.toString(), 663 GenericTestUtils.getMethodName(), "name"); {code} # I don't think we need the implementation details about the Collection type (List). We just need the only element in the collections, don't we? {code} 673 /* get and verify NN dir */ 674 List nameDirs = (List) FSNamesystem.getNamespaceDirs(config); {code} Alternatively: {code} Collection nameDirs = FSNamesystem.getNamespaceDirs(config); assertNotNull(nameDirs); assertTrue(nameDirs.iterator().hasNext()); final URI nameDir = nameDirs.iterator().next(); ... {code} was (Author: liuml07): The patch looks good to me overall. Some minor comments: # Checkstyle warning is related. # MiniDFSCluster can be used in try-catch. Shorter and neater. # I think we can use {{Paths.get()}} for joining path strings. Assuming file path separate is / is not platform independent probably. Also if there are methods in JDK or Apache component that work just fine, I hesitate to use third party libraries. {code} 661 final String nnDirStr = Joiner.on("/").join( 662 hdfsDir.toString(), 663 GenericTestUtils.getMethodName(), "name"); {code} # I don't think we need the implementation details about the Collection type (List). We just need the only element in the collections, don't we? {code} 673 /* get and verify NN dir */ 674 List nameDirs = (List) FSNamesystem.getNamespaceDirs(config); {code} Alternatively: {code} Collection nameDirs = FSNamesystem.getNamespaceDirs(config); assertNotNull(nameDirs); assertTrue(nameDirs.iterator().hasNext()); final URI nameDir = nameDirs.iterator().next(); ... {code} > Add unit tests for NameNode failing to startup when name dir can not be > written > --- > > Key: HDFS-11085 > URL: https://issues.apache.org/jira/browse/HDFS-11085 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: namenode, test >Reporter: Mingliang Liu >Assignee: Xiaobing Zhou > Attachments: HDFS-11085.000.patch > > > This can be placed in {{org.apache.hadoop.hdfs.server.namenode.TestStartup}} > test class. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org