Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-11612

Workaround for Curator's ChildReaper requiring Guava 15+

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.7.0
    • Component/s: None
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      HADOOP-11492 upped the Curator version to 2.7.1, which makes the ChildReaper class use a method that only exists in newer versions of Guava (we have 11.0.2, and it needs 15+). As a workaround, we can copy the ChildReaper class into hadoop-common and make a minor modification to allow it to work with Guava 11.

      The ChildReaper is used by Curator to cleanup old lock znodes. Curator locks are needed by YARN-2942.

      1. HADOOP-11612.001.patch
        8 kB
        Robert Kanter
      2. HADOOP-11612.002.patch
        15 kB
        Robert Kanter
      3. HADOOP-11612.003.patch
        15 kB
        Robert Kanter

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2062 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2062/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2062 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2062/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #112 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/112/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #112 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/112/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #102 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/102/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #102 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/102/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2043 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2043/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2043 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2043/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Yarn-trunk #845 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/845/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #845 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/845/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #111 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/111/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #111 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/111/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7171 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7171/)
          HADOOP-11612. Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7171 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7171/ ) HADOOP-11612 . Workaround for Curator's ChildReaper requiring Guava 15+. (rkanter) (rkanter: rev 6f0133039a064ca82363ac6f29fb255506f31b8a) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/curator/TestChildReaper.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/curator/ChildReaper.java
          Hide
          rkanter Robert Kanter added a comment -

          Thanks for the review Steve Loughran and Tsuyoshi Ozawa; and the suggestion Karthik Kambatla. Committed to trunk and branch-2!

          Show
          rkanter Robert Kanter added a comment - Thanks for the review Steve Loughran and Tsuyoshi Ozawa ; and the suggestion Karthik Kambatla . Committed to trunk and branch-2!
          Hide
          stevel@apache.org Steve Loughran added a comment -

          +1

          Show
          stevel@apache.org Steve Loughran added a comment - +1
          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/12699793/HADOOP-11612.003.patch
          against trunk revision c0d9b93.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5745//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5745//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/12699793/HADOOP-11612.003.patch against trunk revision c0d9b93. +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-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5745//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5745//console This message is automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          I've created HADOOP-11616 to remove the workaround.

          Show
          rkanter Robert Kanter added a comment - I've created HADOOP-11616 to remove the workaround.
          Hide
          rkanter Robert Kanter added a comment -

          The 003 patch makes the regular comments I had on ChildReaper and TestChildReaper into Javadoc comments; it also adds Private and Unstable annotations to ChildReaper.

          Show
          rkanter Robert Kanter added a comment - The 003 patch makes the regular comments I had on ChildReaper and TestChildReaper into Javadoc comments; it also adds Private and Unstable annotations to ChildReaper .
          Hide
          kasha Karthik Kambatla added a comment -

          What's the best way to reflect that?

          Add javadoc comments, mark the class Private-Unstable and doomed, file a follow-up blocker JIRA targeting 3.0.0.

          Show
          kasha Karthik Kambatla added a comment - What's the best way to reflect that? Add javadoc comments, mark the class Private-Unstable and doomed, file a follow-up blocker JIRA targeting 3.0.0.
          Hide
          rkanter Robert Kanter added a comment -

          I thought i'd rebuilt curator against guava < 15 and didn't see problems.

          At the time, I don't think anything was using ChildReaper, so we didn't catch it. I'm planning on using it for something new in YARN-2942. Also, Curator 2.5.0's ChildReaper compiles fine against guava < 15.

          so we should plan to switch back to curator ChildReaper at some point in the future -this patch needs to reflect that.

          I agree. What's the best way to reflect that? I was just going to file a followup JIRA. Or is there an "Upgrade Guava JIRA"?

          if there is more JUnit support in this patch —why not try to get it into curator

          It was mainly changing some imports and not pulling in the iRetry stuff from the parent class. I don't think we can make the test compile against JUnit and what Curator is using (TestNG?). In any case, we want the test in Hadoop for now, to test the copied ChildReaper.

          Show
          rkanter Robert Kanter added a comment - I thought i'd rebuilt curator against guava < 15 and didn't see problems. At the time, I don't think anything was using ChildReaper , so we didn't catch it. I'm planning on using it for something new in YARN-2942 . Also, Curator 2.5.0's ChildReaper compiles fine against guava < 15. so we should plan to switch back to curator ChildReaper at some point in the future -this patch needs to reflect that. I agree. What's the best way to reflect that? I was just going to file a followup JIRA. Or is there an "Upgrade Guava JIRA"? if there is more JUnit support in this patch —why not try to get it into curator It was mainly changing some imports and not pulling in the iRetry stuff from the parent class. I don't think we can make the test compile against JUnit and what Curator is using (TestNG?). In any case, we want the test in Hadoop for now, to test the copied ChildReaper .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Interesting; in HADOOP-11102 I thought i'd rebuilt curator against guava < 15 and didn't see problems.

          1. Guava is pain.
          2. We are going to upgrade in trunk before long, though the price is jenkins isn't going to notice problems in pre-commits.
          3. so we should plan to switch back to curator ChildReaper at some point in the future -this patch needs to reflect that.
          4. if there is more JUnit support in this patch —why not try to get it into curator
          Show
          stevel@apache.org Steve Loughran added a comment - Interesting; in HADOOP-11102 I thought i'd rebuilt curator against guava < 15 and didn't see problems. Guava is pain. We are going to upgrade in trunk before long, though the price is jenkins isn't going to notice problems in pre-commits. so we should plan to switch back to curator ChildReaper at some point in the future -this patch needs to reflect that. if there is more JUnit support in this patch —why not try to get it into curator
          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/12699625/HADOOP-11612.002.patch
          against trunk revision 946456c.

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5742//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5742//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/12699625/HADOOP-11612.002.patch against trunk revision 946456c. +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-common-project/hadoop-common. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5742//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5742//console This message is automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          Thanks for looking at the patch Tsuyoshi Ozawa. That's a good point. In the 002 patch, I've also copied TestChildReaper from Curator, and modified it slightly to work with JUnit.

          Show
          rkanter Robert Kanter added a comment - Thanks for looking at the patch Tsuyoshi Ozawa . That's a good point. In the 002 patch, I've also copied TestChildReaper from Curator, and modified it slightly to work with JUnit.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          Robert Kanter Thank you for taking this issue. I look over your patch. +1 for this change. I think we should add test code, like TestChildReaper, not to introduce bugs with modified ChildReaper.

          Show
          ozawa Tsuyoshi Ozawa added a comment - Robert Kanter Thank you for taking this issue. I look over your patch. +1 for this change. I think we should add test code, like TestChildReaper, not to introduce bugs with modified ChildReaper.
          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/12699584/HADOOP-11612.001.patch
          against trunk revision 1c03376.

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

          -1 tests included. 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. 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 following test timeouts occurred in hadoop-common-project/hadoop-common:

          org.apache.hadoop.http.TestHttpServerLifecycle

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5739//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5739//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/12699584/HADOOP-11612.001.patch against trunk revision 1c03376. +1 @author . The patch does not contain any @author tags. -1 tests included . 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 . 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 following test timeouts occurred in hadoop-common-project/hadoop-common: org.apache.hadoop.http.TestHttpServerLifecycle Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5739//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5739//console This message is automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          99% of the code is a direct copy from Curator. I made some minor adjustments to make it work here; there are comments explaining these changes.

          Show
          rkanter Robert Kanter added a comment - 99% of the code is a direct copy from Curator. I made some minor adjustments to make it work here; there are comments explaining these changes.

            People

            • Assignee:
              rkanter Robert Kanter
              Reporter:
              rkanter Robert Kanter
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development