Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-6198

DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0.0, 2.4.0
    • Fix Version/s: 2.4.1
    • Component/s: datanode
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      For rolling upgrade, the DataNode uses a regex to identify the current block pool directory and replace it with the equivalent trash directory. This regex hard-codes '/' as the file path separator, which doesn't work on Windows.

      1. HDFS-6198.2.patch
        3 kB
        Chris Nauroth
      2. HDFS-6198.1.patch
        0.9 kB
        Chris Nauroth

        Activity

        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #1725 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1725/)
        HDFS-6198. DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627)

        • /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/BlockPoolSliceStorage.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #1725 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1725/ ) HDFS-6198 . DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627 ) /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/BlockPoolSliceStorage.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #1751 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1751/)
        HDFS-6198. DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627)

        • /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/BlockPoolSliceStorage.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1751 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1751/ ) HDFS-6198 . DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627 ) /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/BlockPoolSliceStorage.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #533 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/533/)
        HDFS-6198. DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627)

        • /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/BlockPoolSliceStorage.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #533 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/533/ ) HDFS-6198 . DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627 ) /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/BlockPoolSliceStorage.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Hide
        Chris Nauroth added a comment -

        I committed this to trunk, branch-2 and branch-2.4. Haohui and Arpit, thank you for the code reviews.

        Show
        Chris Nauroth added a comment - I committed this to trunk, branch-2 and branch-2.4. Haohui and Arpit, thank you for the code reviews.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #5466 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5466/)
        HDFS-6198. DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627)

        • /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/BlockPoolSliceStorage.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5466 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5466/ ) HDFS-6198 . DataNode rolling upgrade does not correctly identify current block pool directory and replace with trash on Windows. Contributed by Chris Nauroth. (cnauroth: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1585627 ) /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/BlockPoolSliceStorage.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBlockPoolSliceStorage.java
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 1 new or modified test files.

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

        +1 javadoc. There were no new javadoc warning messages.

        +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 hadoop-hdfs-project/hadoop-hdfs.

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6606//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6606//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/12639058/HDFS-6198.2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +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 hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/6606//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/6606//console This message is automatically generated.
        Hide
        Arpit Agarwal added a comment -

        +1 for the updated patch, pending Jenkins.

        Thanks for finding and fixing it Chris.

        Show
        Arpit Agarwal added a comment - +1 for the updated patch, pending Jenkins. Thanks for finding and fixing it Chris.
        Hide
        Chris Nauroth added a comment -

        Here is patch v2 with the additional test fix mentioned by Arpit.

        Show
        Chris Nauroth added a comment - Here is patch v2 with the additional test fix mentioned by Arpit.
        Hide
        Chris Nauroth added a comment -

        If we were just doing File.separator + "BPstuff" + File.separator, then the regex would be invalid. We'd get an exception while trying to parse the regex. Wrapping it with Pattern.quote is what makes it safe. This injects \Q and \E for literal character quoting. On Linux/Mac, the final pattern is:

        ^(.*)(\Q/\EBP-\d+-\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}-\d+\Q/\E)(current)(.*)$
        

        On Windows, the final pattern is:

        ^(.*)(\Q\\EBP-\d+-\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}-\d+\Q\\E)(current)(.*)$
        

        Thanks for the tip on TestBlockPoolSliceStorage. I'll take a look at that one and provide a new patch.

        Show
        Chris Nauroth added a comment - If we were just doing File.separator + "BPstuff" + File.separator , then the regex would be invalid. We'd get an exception while trying to parse the regex. Wrapping it with Pattern.quote is what makes it safe. This injects \Q and \E for literal character quoting. On Linux/Mac, the final pattern is: ^(.*)(\Q/\EBP-\d+-\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}-\d+\Q/\E)(current)(.*)$ On Windows, the final pattern is: ^(.*)(\Q\\EBP-\d+-\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}-\d+\Q\\E)(current)(.*)$ Thanks for the tip on TestBlockPoolSliceStorage . I'll take a look at that one and provide a new patch.
        Hide
        Arpit Agarwal added a comment -

        Hi Chris, I think we should also fix TestBlockPoolSliceStorage in the same Jira.

        It was intended as a targeted unit test for the regex operations and also uses "/" as the hard-coded separator. I should have known better...

        Show
        Arpit Agarwal added a comment - Hi Chris, I think we should also fix TestBlockPoolSliceStorage in the same Jira. It was intended as a targeted unit test for the regex operations and also uses "/" as the hard-coded separator. I should have known better...
        Hide
        Haohui Mai added a comment -

        Is Pattern.quote(File.separator) going to be {{}} in Windows? If so then the regex should be invalid. Am I correct?

        Show
        Haohui Mai added a comment - Is Pattern.quote(File.separator) going to be {{}} in Windows? If so then the regex should be invalid. Am I correct?
        Hide
        Chris Nauroth added a comment -

        This patch uses the platform-specific file path separator. I've verified that TestDataNodeRollingUpgrade passes on both Mac and Windows after this patch.

        Show
        Chris Nauroth added a comment - This patch uses the platform-specific file path separator. I've verified that TestDataNodeRollingUpgrade passes on both Mac and Windows after this patch.

          People

          • Assignee:
            Chris Nauroth
            Reporter:
            Chris Nauroth
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development