Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-891

DataNode no longer needs to check for dfs.network.script

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 2.0.0-alpha
    • Component/s: datanode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Looking at the code for DataNode.instantiateDataNode())} , I see that it calls {{system.exit(-1) if it is not happy with the configuration

          if (conf.get("dfs.network.script") != null) {
            LOG.error("This configuration for rack identification is not supported" +
                " anymore. RackID resolution is handled by the NameNode.");
            System.exit(-1);
          }
      

      This is excessive. It should throw an exception and let whoever called the method decide how to handle it. The DataNode.main() method will log the exception and exit with a -1 value, but other callers (such as anything using MiniDFSCluster will now see a meaningful message rather than some Junit "tests exited without completing" warning.

      Easy to write a test for the correct behaviour: start a MiniDFSCluster with this configuration set, see what happens.

      1. HDFS-891.patch
        0.9 kB
        Harsh J
      2. hdfs-891.txt
        1 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1054 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1054/)
          HDFS-891. DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1054 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1054/ ) HDFS-891 . DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1019 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1019/)
          HDFS-891. DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762)

          Result = FAILURE
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1019 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1019/ ) HDFS-891 . DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2117 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2117/)
          HDFS-891. DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2117 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2117/ ) HDFS-891 . DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2100 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2100/)
          HDFS-891. DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2100 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2100/ ) HDFS-891 . DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2173 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2173/)
          HDFS-891. DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762)

          Result = SUCCESS
          eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2173 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2173/ ) HDFS-891 . DataNode no longer needs to check for dfs.network.script. Contributed by Harsh J (Revision 1327762) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1327762 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
          Eli Collins made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Target Version/s 2.0.0 [ 12320353 ]
          Fix Version/s 2.0.0 [ 12320353 ]
          Resolution Fixed [ 1 ]
          Hide
          Eli Collins added a comment -

          New jenkins run is clean. No new test is necessary since this just removes the old check. I've committed this and merged to branch-2. Thanks Steve and Harsh!

          Show
          Eli Collins added a comment - New jenkins run is clean. No new test is necessary since this just removes the old check. I've committed this and merged to branch-2. Thanks Steve and Harsh!
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12523249/hdfs-891.txt
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2301//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2301//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12523249/hdfs-891.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2301//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2301//console This message is automatically generated.
          Eli Collins made changes -
          Attachment hdfs-891.txt [ 12523249 ]
          Hide
          Eli Collins added a comment -

          Re-uploading the patch to kick jenkins. I think the previous findbugs/audit warning has to do with us removing a System.exit that is referenced in a findbugs excludes file, want to see what the actual warnings are to be sure. Will hold of committing since we may need to update the findbugs file as well.

          Show
          Eli Collins added a comment - Re-uploading the patch to kick jenkins. I think the previous findbugs/audit warning has to do with us removing a System.exit that is referenced in a findbugs excludes file, want to see what the actual warnings are to be sure. Will hold of committing since we may need to update the findbugs file as well.
          Eli Collins made changes -
          Summary DataNode.instantiateDataNode calls system.exit(-1) if conf.get("dfs.network.script") != null DataNode no longer needs to check for dfs.network.script
          Assignee Harsh J [ qwertymaniac ]
          Affects Version/s 0.23.0 [ 12315571 ]
          Affects Version/s 0.22.0 [ 12314241 ]
          Target Version/s 3.0.0, 2.0.0 [ 12320356, 12320353 ] 2.0.0 [ 12320353 ]
          Hide
          Eli Collins added a comment -

          Agree. +1 patch looks good

          Show
          Eli Collins added a comment - Agree. +1 patch looks good
          Robert Joseph Evans made changes -
          Target Version/s 0.23.1, 0.24.0 [ 12318885, 12317653 ] 2.0.0, 3.0.0 [ 12320353, 12320356 ]
          Hide
          Steve Loughran added a comment -

          +1
          looks good to me -just deletes no longer needed lines

          Show
          Steve Loughran added a comment - +1 looks good to me -just deletes no longer needed lines
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12509791/HDFS-891.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 javadoc. The javadoc tool appears to have generated 21 warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings).

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//artifact/trunk/hadoop-hdfs-project/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12509791/HDFS-891.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javadoc. The javadoc tool appears to have generated 21 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//artifact/trunk/hadoop-hdfs-project/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1766//console This message is automatically generated.
          Harsh J made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Target Version/s 0.24.0, 0.23.1 [ 12317653, 12318885 ]
          Harsh J made changes -
          Attachment HDFS-891.patch [ 12509791 ]
          Hide
          Harsh J added a comment -

          I think we can do away with this check. It is a wrong prop name today, and even if it does exist in the configuration, its not an issue if we already ignore it.

          Patch that gets rid of this legacy check.

          Show
          Harsh J added a comment - I think we can do away with this check. It is a wrong prop name today, and even if it does exist in the configuration, its not an issue if we already ignore it. Patch that gets rid of this legacy check.
          Gavin made changes -
          Reporter Steve Loughran [ steve_l ] Steve Loughran [ stevel@apache.org ]
          steve_l made changes -
          Priority Major [ 3 ] Minor [ 4 ]
          Hide
          steve_l added a comment -

          The lines in question went in as part of the big HADOOP-1985 topology patch, been in since 0.17.

          Show
          steve_l added a comment - The lines in question went in as part of the big HADOOP-1985 topology patch, been in since 0.17.
          steve_l made changes -
          Field Original Value New Value
          Link This issue relates to HADOOP-1985 [ HADOOP-1985 ]
          steve_l created issue -

            People

            • Assignee:
              Harsh J
              Reporter:
              Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development