Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.23.0
    • Component/s: namenode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. h2265_20110816b.patch
      50 kB
      Tsz Wo Nicholas Sze
    2. h2265_20110816.patch
      49 kB
      Tsz Wo Nicholas Sze

      Activity

      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk #756 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/756/)
      HDFS-2265. Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager.

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743
      Files :

      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #756 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/756/ ) HDFS-2265 . Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743 Files : /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hide
      Tsz Wo Nicholas Sze added a comment -

      > The -1 from system test framework was a false negative. ...

      The aop system test was indeed broken but it was not related to this; see HDFS-2270.

      Show
      Tsz Wo Nicholas Sze added a comment - > The -1 from system test framework was a false negative. ... The aop system test was indeed broken but it was not related to this; see HDFS-2270 .
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Common-trunk-Commit #748 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/748/)
      HDFS-2265. Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager.

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743
      Files :

      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #748 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/748/ ) HDFS-2265 . Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743 Files : /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hide
      Tsz Wo Nicholas Sze added a comment -

      Thanks Uma for the review.

      I have committed this.

      Show
      Tsz Wo Nicholas Sze added a comment - Thanks Uma for the review. I have committed this.
      Hide
      Hudson added a comment -

      Integrated in Hadoop-Hdfs-trunk-Commit #838 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/838/)
      HDFS-2265. Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager.

      szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743
      Files :

      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/CHANGES.txt
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
      • /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java
      • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Show
      Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #838 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/838/ ) HDFS-2265 . Remove unnecessary BlockTokenSecretManager fields/methods from BlockManager. szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1158743 Files : /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/blockmanagement/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java /hadoop/common/trunk/hdfs/src/test/hdfs/org/apache/hadoop/hdfs/server/namenode/TestBlockTokenWithDFS.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/server/blockmanagement/BlockManager.java
      Hide
      Tsz Wo Nicholas Sze added a comment -

      Do you mean using try-finally to close the stream? It is fine in unit test since if there is an IOException, the test fails and then the junit framework will clean up the everything. Okay?

      Regarding to private APIs in the tests, let's ignore it for the moment and spend our energy on the main code.

      Show
      Tsz Wo Nicholas Sze added a comment - Do you mean using try-finally to close the stream? It is fine in unit test since if there is an IOException, the test fails and then the junit framework will clean up the everything. Okay? Regarding to private APIs in the tests, let's ignore it for the moment and spend our energy on the main code.
      Hide
      Uma Maheswara Rao G added a comment -

      Hi Nicholas,

      I have gone through the chnages.

      I have one small comment here (This comment is not introduced by this patch)
      1) In TestBlockTokenWithDFS.java
      Since we touched this file, lets clean
      Here looks not closed stream properly.

           private void createFile(FileSystem fs, Path filename) throws IOException {
             FSDataOutputStream out = fs.create(filename);
             out.write(rawData);
             out.close();
           }
           

      One more observation is many places in HDFS tests we used this kind of file creation 'private APIs'.
      DFSTestUtil.java already has file creation APIs. can we move it to that util and use by handling the streams?.

      Other than that patch looks good to me.
      +1

      Show
      Uma Maheswara Rao G added a comment - Hi Nicholas, I have gone through the chnages. I have one small comment here (This comment is not introduced by this patch) 1) In TestBlockTokenWithDFS.java Since we touched this file, lets clean Here looks not closed stream properly. private void createFile(FileSystem fs, Path filename) throws IOException { FSDataOutputStream out = fs.create(filename); out.write(rawData); out.close(); } One more observation is many places in HDFS tests we used this kind of file creation 'private APIs'. DFSTestUtil.java already has file creation APIs. can we move it to that util and use by handling the streams?. Other than that patch looks good to me. +1
      Hide
      Tsz Wo Nicholas Sze added a comment -

      The failed tests are not related to this.

      The -1 from system test framework was a false negative. Everything can be compiled in my machine.

      Show
      Tsz Wo Nicholas Sze added a comment - The failed tests are not related to this. The -1 from system test framework was a false negative. Everything can be compiled in my machine.
      Hide
      Hadoop QA added a comment -

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

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

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

      +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 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 failed these core unit tests:
      org.apache.hadoop.hdfs.server.blockmanagement.TestReplicationPolicy
      org.apache.hadoop.hdfs.server.namenode.TestCheckpoint
      org.apache.hadoop.hdfs.server.namenode.TestNNThroughputBenchmark
      org.apache.hadoop.hdfs.server.namenode.TestValidateConfigurationSettings
      org.apache.hadoop.hdfs.TestHDFSServerPorts

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

      -1 system test framework. The patch failed system test framework compile.

      Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//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/12490621/h2265_20110816b.patch against trunk revision 1158025. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +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 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 failed these core unit tests: org.apache.hadoop.hdfs.server.blockmanagement.TestReplicationPolicy org.apache.hadoop.hdfs.server.namenode.TestCheckpoint org.apache.hadoop.hdfs.server.namenode.TestNNThroughputBenchmark org.apache.hadoop.hdfs.server.namenode.TestValidateConfigurationSettings org.apache.hadoop.hdfs.TestHDFSServerPorts +1 contrib tests. The patch passed contrib unit tests. -1 system test framework. The patch failed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1108//console This message is automatically generated.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h2265_20110816b.patch: removed synchronized from BlockTokenSecretManager.updateKeys(long) and changed the test to use junit 4.

      Show
      Tsz Wo Nicholas Sze added a comment - h2265_20110816b.patch: removed synchronized from BlockTokenSecretManager.updateKeys(long) and changed the test to use junit 4.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h2265_20110816.patch: 1st patch.

      Show
      Tsz Wo Nicholas Sze added a comment - h2265_20110816.patch: 1st patch.

        People

        • Assignee:
          Tsz Wo Nicholas Sze
          Reporter:
          Tsz Wo Nicholas Sze
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development