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

TestFSNameSystemMBean should wait until JMX cache is cleared

    Details

    • Hadoop Flags:
      Reviewed

      Description

      TestFSNamesystemMBean#testWithFSNamesystemWriteLock and #testWithFSEditLogLock get metrics after locking FSNameSystem/FSEditLog, but when the metrics are cached, the tests success even if the metrics acquire the locks. The tests should wait until the cache is cleared.
      This issue was reported by Erik Krogen in HDFS-11180.

      1. HDFS-11290.001.patch
        2 kB
        Erik Krogen
      2. HDFS-11290.000.patch
        2 kB
        Erik Krogen

        Issue Links

          Activity

          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - 2.8.1 became a security release. Moving fix-version to 2.8.2 after the fact.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Backported this to branch-2.8, branch-2.7, and branch-2.6 to verify that HDFS-11352 fixes the potential deadlock.

          Show
          ajisakaa Akira Ajisaka added a comment - Backported this to branch-2.8, branch-2.7, and branch-2.6 to verify that HDFS-11352 fixes the potential deadlock.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11133 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11133/)
          HDFS-11290. TestFSNameSystemMBean should wait until JMX cache is (aajisaka: rev b1a9ec856b572894e769f052aea2340fc3f23c78)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemMBean.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11133 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11133/ ) HDFS-11290 . TestFSNameSystemMBean should wait until JMX cache is (aajisaka: rev b1a9ec856b572894e769f052aea2340fc3f23c78) (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystemMBean.java
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Committed this to trunk and branch-2. Thanks Erik Krogen for the contribution!

          Show
          ajisakaa Akira Ajisaka added a comment - Committed this to trunk and branch-2. Thanks Erik Krogen for the contribution!
          Hide
          ajisakaa Akira Ajisaka added a comment -

          +1

          Show
          ajisakaa Akira Ajisaka added a comment - +1
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 13m 37s trunk passed
          +1 compile 0m 48s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 52s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 48s trunk passed
          +1 javadoc 0m 43s trunk passed
          +1 mvninstall 0m 49s the patch passed
          +1 compile 0m 45s the patch passed
          +1 javac 0m 45s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 51s the patch passed
          +1 javadoc 0m 36s the patch passed
          +1 unit 101m 15s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          127m 8s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11290
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12847857/HDFS-11290.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5379a8265a88 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 78b487b
          Default Java 1.8.0_111
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18190/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18190/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 24s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 13m 37s trunk passed +1 compile 0m 48s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 52s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 48s trunk passed +1 javadoc 0m 43s trunk passed +1 mvninstall 0m 49s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 51s the patch passed +1 javadoc 0m 36s the patch passed +1 unit 101m 15s hadoop-hdfs in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 127m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11290 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12847857/HDFS-11290.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5379a8265a88 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 78b487b Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18190/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18190/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xkrogen Erik Krogen added a comment -

          Thanks for the pointer Akira Ajisaka! I missed that completely. I've attached a v001 patch with the suggested change.

          Show
          xkrogen Erik Krogen added a comment - Thanks for the pointer Akira Ajisaka ! I missed that completely. I've attached a v001 patch with the suggested change.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Thanks Erik Krogen for providing a patch. Mostly looks good to me.

          +    new ConfigBuilder().add("namenode.period", jmxCachePeriod)
          +        .save("hadoop-metrics2.properties");
          

          Would you use TestMetricsConfig.getTestFilename to generate the metrics config file to hadoop-hdfs-project/hadoop-hdfs/target/test-class? I'm +1 if that is addressed.

          Show
          ajisakaa Akira Ajisaka added a comment - Thanks Erik Krogen for providing a patch. Mostly looks good to me. + new ConfigBuilder().add( "namenode.period" , jmxCachePeriod) + .save( "hadoop-metrics2.properties" ); Would you use TestMetricsConfig.getTestFilename to generate the metrics config file to hadoop-hdfs-project/hadoop-hdfs/target/test-class ? I'm +1 if that is addressed.
          Hide
          xkrogen Erik Krogen added a comment -

          Attaching a patch which uses a Thread.sleep(1000) to force the JMX cache to expire. The way the metrics system is currently set up you can't set a JMX cache TTL below 1 second; setting the period to 0 causes an IllegalArgumentException in MetricsSystemImpl#startTimer()... Open to suggestions on a better way to do this, but it's a little tricky since it's an integration-style test so I can't easily hook into/modify the metrics system. I verified that without this patch it's possible for testWithFSEditLogLock to pass (as well as testWithFSNamesystemWriteLock if I increase the JMX cache TTL to 20 seconds, so it's basically performance-dependent) if I change FSNamesystem#getLastWrittenTransactionId to the following, which should cause both to fail:

            public long getLastWrittenTransactionId() {
              readLock();
              readUnlock();
              return getEditLog().getLastWrittenTxId();
            }
          

          Akira Ajisaka, any thoughts?

          Show
          xkrogen Erik Krogen added a comment - Attaching a patch which uses a Thread.sleep(1000) to force the JMX cache to expire. The way the metrics system is currently set up you can't set a JMX cache TTL below 1 second; setting the period to 0 causes an IllegalArgumentException in MetricsSystemImpl#startTimer() ... Open to suggestions on a better way to do this, but it's a little tricky since it's an integration-style test so I can't easily hook into/modify the metrics system. I verified that without this patch it's possible for testWithFSEditLogLock to pass (as well as testWithFSNamesystemWriteLock if I increase the JMX cache TTL to 20 seconds, so it's basically performance-dependent) if I change FSNamesystem#getLastWrittenTransactionId to the following, which should cause both to fail: public long getLastWrittenTransactionId() { readLock(); readUnlock(); return getEditLog().getLastWrittenTxId(); } Akira Ajisaka , any thoughts?

            People

            • Assignee:
              xkrogen Erik Krogen
              Reporter:
              ajisakaa Akira Ajisaka
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development