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

Use try with resources in DataStorage and Storage

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: datanode
    • Labels:
      None
    • Target Version/s:

      Description

      We have some old-style try/finally to close files in DataStorage and Storage, let's update them.

      Also a few small cleanups:

      • Actually check that tryLock returns a FileLock in isPreUpgradableLayout
      • Remove unused parameter from writeProperties
      • Add braces for one-line if statements per coding style
      1. HDFS-8546.001.patch
        5 kB
        Andrew Wang
      2. HDFS-8546.002.patch
        6 kB
        Andrew Wang
      3. HDFS-8546.003.patch
        6 kB
        Andrew Wang
      4. HDFS-8546.004.patch
        6 kB
        Andrew Wang
      5. HDFS-8546.005.patch
        6 kB
        Andrew Wang
      6. HDFS-8546.006.patch
        6 kB
        Andrew Wang

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2186 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2186/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2186 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2186/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk #2168 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2168/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2168 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2168/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #238 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/238/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #238 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/238/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #229 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/229/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #229 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/229/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #970 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/970/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #970 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/970/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #240 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/240/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #240 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/240/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8070 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8070/)
        HDFS-8546. Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8070 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8070/ ) HDFS-8546 . Use try with resources in DataStorage and Storage. (wang: rev 1403b84b122fb76ef2b085a728b5402c32499c1f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataStorage.java hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/Storage.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        andrew.wang Andrew Wang added a comment -

        Thanks Lei (Eddy) Xu for final review, Tsz Wo Nicholas Sze for earlier ones. Committed to trunk and branch-2.

        Show
        andrew.wang Andrew Wang added a comment - Thanks Lei (Eddy) Xu for final review, Tsz Wo Nicholas Sze for earlier ones. Committed to trunk and branch-2.
        Hide
        eddyxu Lei (Eddy) Xu added a comment -

        +1. Thanks Andrew Wang.

        Show
        eddyxu Lei (Eddy) Xu added a comment - +1. Thanks Andrew Wang .
        Hide
        andrew.wang Andrew Wang added a comment -

        Ping Tsz Wo Nicholas Sze last review?

        Show
        andrew.wang Andrew Wang added a comment - Ping Tsz Wo Nicholas Sze last review?
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 17m 57s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 7m 45s There were no new javac warning messages.
        +1 javadoc 9m 55s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 2m 18s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 36s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 3m 24s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 18s Pre-build of native portion
        -1 hdfs tests 168m 10s Tests failed in hadoop-hdfs.
            215m 23s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.namenode.ha.TestPipelinesFailover
        Timed out tests org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistLockedMemory



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12739899/HDFS-8546.006.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / b039e69
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11371/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11371/testReport/
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11371/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 17m 57s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 7m 45s There were no new javac warning messages. +1 javadoc 9m 55s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 2m 18s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 24s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 18s Pre-build of native portion -1 hdfs tests 168m 10s Tests failed in hadoop-hdfs.     215m 23s   Reason Tests Failed unit tests hadoop.hdfs.server.namenode.ha.TestPipelinesFailover Timed out tests org.apache.hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistLockedMemory Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12739899/HDFS-8546.006.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b039e69 hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11371/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11371/testReport/ Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11371/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        Here's a patch which adds LOG.errors with the path, LMK what you think. I don't have any additional insight about why OverlappingLockException doesn't take a String parameter.

        Show
        andrew.wang Andrew Wang added a comment - Here's a patch which adds LOG.errors with the path, LMK what you think. I don't have any additional insight about why OverlappingLockException doesn't take a String parameter.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        -1 pre-patch 15m 17s Findbugs (version ) appears to be broken on trunk.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 7m 36s There were no new javac warning messages.
        +1 javadoc 9m 35s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 0m 58s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 37s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 3m 15s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 15s Pre-build of native portion
        +1 hdfs tests 161m 49s Tests passed in hadoop-hdfs.
            204m 22s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12739361/HDFS-8546.005.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / eef7b50
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11335/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11335/testReport/
        Java 1.7.0_55
        uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11335/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 15m 17s Findbugs (version ) appears to be broken on trunk. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 7m 36s There were no new javac warning messages. +1 javadoc 9m 35s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 0m 58s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 37s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 15s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 15s Pre-build of native portion +1 hdfs tests 161m 49s Tests passed in hadoop-hdfs.     204m 22s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12739361/HDFS-8546.005.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / eef7b50 hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11335/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11335/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11335/console This message was automatically generated.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -

        > ... OverlappingFileLockException is a JDK class that doesn't have a constructor that takes a String, so could not address that one. ...

        Do you know if there a reason that message cannot be set in OverlappingFileLockException?

        It is hard to debug if we see OverlappingFileLockException but the path info is missing. How about either add a LOG.error message, or use a different exception, say IllegalStateException (a base class of OverlappingFileLockException) in order to show the file oldF?

        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - > ... OverlappingFileLockException is a JDK class that doesn't have a constructor that takes a String, so could not address that one. ... Do you know if there a reason that message cannot be set in OverlappingFileLockException? It is hard to debug if we see OverlappingFileLockException but the path info is missing. How about either add a LOG.error message, or use a different exception, say IllegalStateException (a base class of OverlappingFileLockException) in order to show the file oldF?
        Hide
        andrew.wang Andrew Wang added a comment -

        Good catch, made that last rev a little too quickly. Moved oldLock into the try.

        Show
        andrew.wang Andrew Wang added a comment - Good catch, made that last rev a little too quickly. Moved oldLock into the try.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -
        +    try (RandomAccessFile oldFile = new RandomAccessFile(oldF, "rws")) {
        +      FileLock oldLock = oldFile.getChannel().tryLock();
        

        The oldLock above should be inside try(..).

        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - + try (RandomAccessFile oldFile = new RandomAccessFile(oldF, "rws" )) { + FileLock oldLock = oldFile.getChannel().tryLock(); The oldLock above should be inside try(..).
        Hide
        andrew.wang Andrew Wang added a comment -

        Tsz Wo Nicholas Sze any further review comments for latest rev? Thanks.

        Show
        andrew.wang Andrew Wang added a comment - Tsz Wo Nicholas Sze any further review comments for latest rev? Thanks.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 17m 45s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 7m 29s There were no new javac warning messages.
        +1 javadoc 9m 38s There were no new javadoc warning messages.
        +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 2m 18s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 33s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 15s Pre-build of native portion
        +1 hdfs tests 161m 46s Tests passed in hadoop-hdfs.
            208m 4s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12738402/HDFS-8546.004.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 0e80d51
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11272/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11272/testReport/
        Java 1.7.0_55
        uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11272/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 17m 45s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 7m 29s There were no new javac warning messages. +1 javadoc 9m 38s There were no new javadoc warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 2m 18s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 33s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 15s Pre-build of native portion +1 hdfs tests 161m 46s Tests passed in hadoop-hdfs.     208m 4s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12738402/HDFS-8546.004.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 0e80d51 hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11272/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11272/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11272/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        TY for the review again, I removed the extra finally. OverlappingFileLockException is a JDK class that doesn't have a constructor that takes a String, so could not address that one. Fixed the checkstyle issues too.

        Show
        andrew.wang Andrew Wang added a comment - TY for the review again, I removed the extra finally. OverlappingFileLockException is a JDK class that doesn't have a constructor that takes a String, so could not address that one. Fixed the checkstyle issues too.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 17m 42s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 7m 30s There were no new javac warning messages.
        +1 javadoc 9m 37s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 2m 14s The applied patch generated 4 new checkstyle issues (total was 136, now 137).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 32s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 13s Pre-build of native portion
        -1 hdfs tests 161m 21s Tests failed in hadoop-hdfs.
            207m 28s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.namenode.TestCheckpoint



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12738106/HDFS-8546.003.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 71de367
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11256/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11256/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11256/testReport/
        Java 1.7.0_55
        uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11256/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 17m 42s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 7m 30s There were no new javac warning messages. +1 javadoc 9m 37s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 14s The applied patch generated 4 new checkstyle issues (total was 136, now 137). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 20s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 13s Pre-build of native portion -1 hdfs tests 161m 21s Tests failed in hadoop-hdfs.     207m 28s   Reason Tests Failed unit tests hadoop.hdfs.server.namenode.TestCheckpoint Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12738106/HDFS-8546.003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 71de367 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11256/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11256/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11256/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11256/console This message was automatically generated.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -
        • There is still a double try-statements in isPreUpgradableLayout. Note that FileLock is AutoCloseable
        • We should add a message including when creating OverlappingFileLockException.
        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - There is still a double try-statements in isPreUpgradableLayout. Note that FileLock is AutoCloseable We should add a message including when creating OverlappingFileLockException.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 17m 39s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 7m 28s There were no new javac warning messages.
        +1 javadoc 9m 29s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 2m 11s The applied patch generated 4 new checkstyle issues (total was 135, now 136).
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 32s mvn install still works.
        +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse.
        +1 findbugs 3m 13s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 14s Pre-build of native portion
        +1 hdfs tests 157m 43s Tests passed in hadoop-hdfs.
            203m 26s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12738057/HDFS-8546.002.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / bc11e15
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11250/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11250/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11250/testReport/
        Java 1.7.0_55
        uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11250/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 17m 39s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 7m 28s There were no new javac warning messages. +1 javadoc 9m 29s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 11s The applied patch generated 4 new checkstyle issues (total was 135, now 136). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 32s mvn install still works. +1 eclipse:eclipse 0m 32s The patch built with eclipse:eclipse. +1 findbugs 3m 13s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 14s Pre-build of native portion +1 hdfs tests 157m 43s Tests passed in hadoop-hdfs.     203m 26s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12738057/HDFS-8546.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / bc11e15 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11250/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11250/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11250/testReport/ Java 1.7.0_55 uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11250/console This message was automatically generated.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 21m 58s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s 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 javac 9m 31s There were no new javac warning messages.
        +1 javadoc 10m 53s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 2m 24s The applied patch generated 1 new checkstyle issues (total was 135, now 134).
        -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix.
        +1 install 1m 36s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 3m 16s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 19s Pre-build of native portion
        +1 hdfs tests 162m 10s Tests passed in hadoop-hdfs.
            216m 5s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12738052/HDFS-8546.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / bc11e15
        checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt
        whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/whitespace.txt
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11249/testReport/
        Java 1.7.0_55
        uname Linux asf904.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11249/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 21m 58s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s 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 javac 9m 31s There were no new javac warning messages. +1 javadoc 10m 53s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 2m 24s The applied patch generated 1 new checkstyle issues (total was 135, now 134). -1 whitespace 0m 0s The patch has 3 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 3m 16s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 19s Pre-build of native portion +1 hdfs tests 162m 10s Tests passed in hadoop-hdfs.     216m 5s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12738052/HDFS-8546.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / bc11e15 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/diffcheckstylehadoop-hdfs.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/whitespace.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/11249/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/11249/testReport/ Java 1.7.0_55 uname Linux asf904.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/11249/console This message was automatically generated.
        Hide
        andrew.wang Andrew Wang added a comment -

        Thanks for reviewing Nicholas. I did that in writeProperties because I didn't want to reorder around the file.seek, but on second look it seems safe. New rev puts both in the same try rather than using two.

        Show
        andrew.wang Andrew Wang added a comment - Thanks for reviewing Nicholas. I did that in writeProperties because I didn't want to reorder around the file.seek , but on second look it seems safe. New rev puts both in the same try rather than using two.
        Hide
        szetszwo Tsz Wo Nicholas Sze added a comment -

        When using try-with-resources, we don't need double try-statements in our case.

        Show
        szetszwo Tsz Wo Nicholas Sze added a comment - When using try-with-resources, we don't need double try-statements in our case.
        Hide
        andrew.wang Andrew Wang added a comment -

        Looking at it more, I realized that copying the dncp log over on upgrade is unnecessary, since the O(1) BlockScanner rewrite doesn't use it. The dncp log will still be in the previous directory if the DN is rolled back, but there's no need to keep carrying it forward.

        Show
        andrew.wang Andrew Wang added a comment - Looking at it more, I realized that copying the dncp log over on upgrade is unnecessary, since the O(1) BlockScanner rewrite doesn't use it. The dncp log will still be in the previous directory if the DN is rolled back, but there's no need to keep carrying it forward.
        Hide
        andrew.wang Andrew Wang added a comment -

        Patch attached

        Show
        andrew.wang Andrew Wang added a comment - Patch attached

          People

          • Assignee:
            andrew.wang Andrew Wang
            Reporter:
            andrew.wang Andrew Wang
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development