Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-7610

Fix removal of dynamically added DN volumes

    Details

    • Target Version/s:

      Description

      In the hot swap feature, FsDatasetImpl#addVolume uses the base volume dir (e.g. "/foo/data0", instead of volume's current dir "/foo/data/current" to construct FsVolumeImpl. As a result, DataNode can not remove this newly added volume, because its FsVolumeImpl#getBasePath returns "/foo".

      1. HDFS-7610.001.patch
        8 kB
        Lei (Eddy) Xu
      2. HDFS-7610.000.patch
        9 kB
        Lei (Eddy) Xu

        Activity

        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        This patch changes:

        1. Use StorageDirectory.getCurrentDir to construct FsVolumeImpl when adding the volume dynamically.
        2. Changed removeVolume() to compare volumes using File#getCanonicalFile().
        3. Added a test case to verify that a newly added volume can be located and removed.

        Show
        eddyxu Lei (Eddy) Xu added a comment - This patch changes: 1. Use StorageDirectory.getCurrentDir to construct FsVolumeImpl when adding the volume dynamically. 2. Changed removeVolume() to compare volumes using File#getCanonicalFile() . 3. Added a test case to verify that a newly added volume can be located and removed.
        Hide
        hadoopqa Hadoop QA added a comment -

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

        +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 appears to introduce 1 new Findbugs (version 2.0.3) warnings.

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

        -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.qjournal.TestSecureNNWithQJM

        The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.TestFileCreation

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12692341/HDFS-7610.000.patch against trunk revision 6464a89. +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 appears to introduce 1 new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.qjournal.TestSecureNNWithQJM The following test timeouts occurred in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestFileCreation Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//artifact/patchprocess/newPatchFindbugsWarningshadoop-hdfs.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9215//console This message is automatically generated.
        Hide
        cmccabe Colin P. McCabe added a comment -
        +      try {
        +        volumeSet.add(sl.getFile().getCanonicalFile());
        +      } catch (IOException e) {
        +        // Thrown because File#getCanoicalFile(). Ignored.
        +      }
        

        We can't ignore exceptions like this. Anyway, I don't think we need the "canonical" filename anyway. If someone is playing games with symlinks, that's their own problem, not ours. Just get the absolute pathname, an operation that can't fail.

        -    Set<String> expectedVolumes = new HashSet<String>();
        +    Set<String> expectedVolumes = new HashSet<>();
        

        Changes like this are just churn that makes it harder to read the diff, so let's not

        thanks

        Show
        cmccabe Colin P. McCabe added a comment - + try { + volumeSet.add(sl.getFile().getCanonicalFile()); + } catch (IOException e) { + // Thrown because File#getCanoicalFile(). Ignored. + } We can't ignore exceptions like this. Anyway, I don't think we need the "canonical" filename anyway. If someone is playing games with symlinks, that's their own problem, not ours. Just get the absolute pathname, an operation that can't fail. - Set< String > expectedVolumes = new HashSet< String >(); + Set< String > expectedVolumes = new HashSet<>(); Changes like this are just churn that makes it harder to read the diff, so let's not thanks
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        Colin P. McCabe Thanks for reviewing.

        I have updated the patch to address your comments above.

        Show
        eddyxu Lei (Eddy) Xu added a comment - Colin P. McCabe Thanks for reviewing. I have updated the patch to address your comments above.
        Hide
        hadoopqa Hadoop QA added a comment -

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

        +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 2.0.3) 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.

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

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12693398/HDFS-7610.001.patch against trunk revision dd0228b. +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 2.0.3) 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. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/9281//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/9281//console This message is automatically generated.
        Hide
        cmccabe Colin P. McCabe added a comment -

        +1. Thanks, Lei (Eddy) Xu.

        Show
        cmccabe Colin P. McCabe added a comment - +1. Thanks, Lei (Eddy) Xu .
        Hide
        cmccabe Colin P. McCabe added a comment -

        committed, thanks!

        Show
        cmccabe Colin P. McCabe added a comment - committed, thanks!
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #6899 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6899/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #6899 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6899/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #814 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/814/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #814 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/814/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #80 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/80/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #80 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/80/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #77 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/77/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #77 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/77/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk #2012 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2012/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2012 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2012/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #81 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/81/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #81 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/81/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2031 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2031/)
        HDFS-7610. Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java
          HDFS-7610. Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b)
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Mapreduce-trunk #2031 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2031/ ) HDFS-7610 . Fix removal of dynamically added DN volumes (Lei (Eddy) Xu via Colin P. McCabe) (cmccabe: rev a17584936cc5141e3f5612ac3ecf35e27968e439) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/TestFsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsVolumeList.java HDFS-7610 . Add CHANGES.txt (cmccabe: rev a1222784fbc4b0000b51be96586ec2ae7098264b) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        Sangjin Lee backported this to 2.6.1, after fixing minor issues (with java diamond operator). I just pushed the commit to 2.6.1 after running compilation and TestFsDatasetImpl which changed in the patch.

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - Sangjin Lee backported this to 2.6.1, after fixing minor issues (with java diamond operator). I just pushed the commit to 2.6.1 after running compilation and TestFsDatasetImpl which changed in the patch.

          People

          • Assignee:
            eddyxu Lei (Eddy) Xu
            Reporter:
            eddyxu Lei (Eddy) Xu
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development