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

Add LambdaTestUtils class for tests; fix eventual consistency problem in contract test setup

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: test
    • Labels:
      None

      Description

      To make our tests robust against timing problems and eventual consistent stores, we need to do more spin & wait for state.

      We have some code in GenericTestUtils.waitFor to await a condition being met, but the predicate it calls doesn't throw exceptions, there's no way for a probe to throw an exception, and all you get is the eventual "timed out" message.

      We can do better, and in closure-ready languages (scala & scalatest, groovy and some slider code) we've examples to follow. Some of that work has been reimplemented slightly in S3ATestUtils.eventually

      I propose adding a class in the test tree, Eventually to be a successor/replacement for these.

      1. has an eventually/waitfor operation taking a predicate that throws an exception
      2. has an "evaluate" exception which tries to evaluate an answer until the operation stops raising an exception. (again, from scalatest)
      3. plugin backoff strategies (from Scalatest; lets you do exponential as well as linear)
      4. option of adding a special handler to generate the failure exception (e.g. run more detailed diagnostics for the exception text, etc).
      5. be Java 8 lambda expression friendly
      6. be testable and tested itself.
      1. HADOOP-13716-001.patch
        18 kB
        Steve Loughran
      2. HADOOP-13716-002.patch
        30 kB
        Steve Loughran
      3. HADOOP-13716-003.patch
        35 kB
        Steve Loughran
      4. HADOOP-13716-005.patch
        40 kB
        Steve Loughran
      5. HADOOP-13716-006.patch
        40 kB
        Steve Loughran
      6. HADOOP-13716-branch-2-004.patch
        31 kB
        Steve Loughran

        Issue Links

          Activity

          Hide
          stevel@apache.org Steve Loughran added a comment - - edited

          example from a test

              int reps = eventually(
                  () -> ++count == 4,
                  50,
                  () -> 10,
                  (timeout, ex) -> new Exception("timeout after " + count)
              );
          

          This will evaluate the first closure until it succeeds or the total wait time exceeds 50; the operation for interval calculation is always 10. Returns the number of iterations it took.

          Another example, evaluate a closure until it succeeds. Here it doesn't; the exception caught on the final unsuccessful iteration is rethrown.

            @Test
            public void testEvalLambdaFailures() throws Throwable {
              try {
                evaluate(() -> {
                      throw new IOException("oops");
                    },
                    TIMEOUT,
                    retry);
                fail("should not have got here");
              } catch (IOException expected) {
                assertExceptionContains("oops", expected);
              }
            }
          

          As well as being Lambda friendly, the patch is just invoking Callable<T>, so works with Java & and normal interface/implementation classes; that's how I plan to initially use it for branch-2 code. I just want to be confident we have something which will move to lambda-expressions with ease.

          Also, I've added a FailFastException, similar to that in S3ATestUtils. If a closure throws that: no attempt to retry. Code that knows what it is doing can use that to bail out fast on a condition which will never be met.

          I'm looking at doing some S3A work, in particular handling inconsistency problems in some of the FS contract root tests, though I think someone ought to be able to migrate waitFor() code to it when they want better exception propagation, failure handling or alternative backoff strategies

          Show
          stevel@apache.org Steve Loughran added a comment - - edited example from a test int reps = eventually( () -> ++count == 4, 50, () -> 10, (timeout, ex) -> new Exception( "timeout after " + count) ); This will evaluate the first closure until it succeeds or the total wait time exceeds 50; the operation for interval calculation is always 10. Returns the number of iterations it took. Another example, evaluate a closure until it succeeds. Here it doesn't; the exception caught on the final unsuccessful iteration is rethrown. @Test public void testEvalLambdaFailures() throws Throwable { try { evaluate(() -> { throw new IOException( "oops" ); }, TIMEOUT, retry); fail( "should not have got here" ); } catch (IOException expected) { assertExceptionContains( "oops" , expected); } } As well as being Lambda friendly, the patch is just invoking Callable<T> , so works with Java & and normal interface/implementation classes; that's how I plan to initially use it for branch-2 code. I just want to be confident we have something which will move to lambda-expressions with ease. Also, I've added a FailFastException, similar to that in S3ATestUtils. If a closure throws that: no attempt to retry. Code that knows what it is doing can use that to bail out fast on a condition which will never be met. I'm looking at doing some S3A work, in particular handling inconsistency problems in some of the FS contract root tests, though I think someone ought to be able to migrate waitFor() code to it when they want better exception propagation, failure handling or alternative backoff strategies
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 001.

          The production code is Java7+; the tests are of both java7 and java 8 code...for backporting to java 8 you'd just pull those test cases with the word "lambda" in.

          Because this is needed to improve some of the FS contract tests, they should be a PoC that the API works —I'd be targeting Java 7 there.

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 001. The production code is Java7+; the tests are of both java7 and java 8 code...for backporting to java 8 you'd just pull those test cases with the word "lambda" in. Because this is needed to improve some of the FS contract tests, they should be a PoC that the API works —I'd be targeting Java 7 there.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 37s trunk passed
          +1 compile 6m 47s trunk passed
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 55s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 19s trunk passed
          +1 javadoc 0m 42s trunk passed
          +1 mvninstall 0m 36s the patch passed
          +1 compile 6m 45s the patch passed
          +1 javac 6m 45s the patch passed
          -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 13 new + 0 unchanged - 0 fixed = 13 total (was 0)
          +1 mvnsite 0m 54s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 26s the patch passed
          +1 javadoc 0m 41s the patch passed
          +1 unit 8m 8s hadoop-common in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          38m 1s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832968/HADOOP-13716-001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e3d138890d49 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6476934
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/console
          Powered by Apache Yetus 0.4.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 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 37s trunk passed +1 compile 6m 47s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 19s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 6m 45s the patch passed +1 javac 6m 45s the patch passed -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 13 new + 0 unchanged - 0 fixed = 13 total (was 0) +1 mvnsite 0m 54s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 26s the patch passed +1 javadoc 0m 41s the patch passed +1 unit 8m 8s hadoop-common in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 38m 1s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832968/HADOOP-13716-001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e3d138890d49 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6476934 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10748/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-13716 patch 002; tunes the Eventually class; adds more tests. Migrate ITestS3AFailureHandling to the class, cutting its predecessor in S3ATestUtil. Use it in AbstractContractRootDirectoryTest to verify the API is workable in real test code

          All java 8 code is in the TestEventually test suite; not in the applied uses

          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-13716 patch 002; tunes the Eventually class; adds more tests. Migrate ITestS3AFailureHandling to the class, cutting its predecessor in S3ATestUtil. Use it in AbstractContractRootDirectoryTest to verify the API is workable in real test code All java 8 code is in the TestEventually test suite; not in the applied uses
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 1m 38s Maven dependency ordering for branch
          +1 mvninstall 7m 21s trunk passed
          +1 compile 7m 15s trunk passed
          +1 checkstyle 1m 28s trunk passed
          +1 mvnsite 1m 26s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 1m 54s trunk passed
          +1 javadoc 1m 2s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 0s the patch passed
          +1 compile 7m 11s the patch passed
          +1 javac 7m 11s the patch passed
          -0 checkstyle 1m 30s root: The patch generated 12 new + 25 unchanged - 0 fixed = 37 total (was 25)
          +1 mvnsite 1m 25s the patch passed
          +1 mvneclipse 0m 35s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 12s the patch passed
          +1 javadoc 1m 7s the patch passed
          +1 unit 8m 11s hadoop-common in the patch passed.
          +1 unit 0m 28s hadoop-aws in the patch passed.
          +1 asflicense 0m 27s The patch does not generate ASF License warnings.
          70m 49s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833105/HADOOP-13716-002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 31cd3aac5d20 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 901eca0
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/console
          Powered by Apache Yetus 0.4.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 19s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 1m 38s Maven dependency ordering for branch +1 mvninstall 7m 21s trunk passed +1 compile 7m 15s trunk passed +1 checkstyle 1m 28s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 1m 54s trunk passed +1 javadoc 1m 2s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 7m 11s the patch passed +1 javac 7m 11s the patch passed -0 checkstyle 1m 30s root: The patch generated 12 new + 25 unchanged - 0 fixed = 37 total (was 25) +1 mvnsite 1m 25s the patch passed +1 mvneclipse 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 12s the patch passed +1 javadoc 1m 7s the patch passed +1 unit 8m 11s hadoop-common in the patch passed. +1 unit 0m 28s hadoop-aws in the patch passed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 70m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833105/HADOOP-13716-002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 31cd3aac5d20 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 901eca0 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10759/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-13716 patch 003:

          • expanded goal of being set of lambda expressions, rather than just eventually() & evaluate()
          • added intercept() to intercept exceptions, optionally look for text. Self testing!
          • renamed to LambdaTestUtils to emphasise role and scope
          • best effort at addressing codestyle warnings; IDE may still be getting indentation wrong.

          Again, designed to work with Java 7 alongside java 8, just works better when you can do closures without writing anonymous inner classes

          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-13716 patch 003: expanded goal of being set of lambda expressions, rather than just eventually() & evaluate() added intercept() to intercept exceptions, optionally look for text. Self testing! renamed to LambdaTestUtils to emphasise role and scope best effort at addressing codestyle warnings; IDE may still be getting indentation wrong. Again, designed to work with Java 7 alongside java 8, just works better when you can do closures without writing anonymous inner classes
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ps: s3a ireland tests working with this;

          Show
          stevel@apache.org Steve Loughran added a comment - ps: s3a ireland tests working with this;
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 0m 18s Maven dependency ordering for branch
          +1 mvninstall 6m 38s trunk passed
          +1 compile 6m 50s trunk passed
          +1 checkstyle 1m 26s trunk passed
          +1 mvnsite 1m 20s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 1m 48s trunk passed
          +1 javadoc 1m 1s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 0m 58s the patch passed
          +1 compile 6m 46s the patch passed
          -1 javac 6m 46s root generated 1 new + 709 unchanged - 1 fixed = 710 total (was 710)
          -0 checkstyle 1m 30s root: The patch generated 11 new + 25 unchanged - 0 fixed = 36 total (was 25)
          +1 mvnsite 1m 24s the patch passed
          +1 mvneclipse 0m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 15s the patch passed
          +1 javadoc 1m 6s the patch passed
          +1 unit 8m 38s hadoop-common in the patch passed.
          +1 unit 0m 28s hadoop-aws in the patch passed.
          +1 asflicense 0m 27s The patch does not generate ASF License warnings.
          67m 55s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833118/HADOOP-13716-003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2cc204ff8944 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 9454dc5
          Default Java 1.8.0_101
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/console
          Powered by Apache Yetus 0.4.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 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 0m 18s Maven dependency ordering for branch +1 mvninstall 6m 38s trunk passed +1 compile 6m 50s trunk passed +1 checkstyle 1m 26s trunk passed +1 mvnsite 1m 20s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 1m 48s trunk passed +1 javadoc 1m 1s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 0m 58s the patch passed +1 compile 6m 46s the patch passed -1 javac 6m 46s root generated 1 new + 709 unchanged - 1 fixed = 710 total (was 710) -0 checkstyle 1m 30s root: The patch generated 11 new + 25 unchanged - 0 fixed = 36 total (was 25) +1 mvnsite 1m 24s the patch passed +1 mvneclipse 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 15s the patch passed +1 javadoc 1m 6s the patch passed +1 unit 8m 38s hadoop-common in the patch passed. +1 unit 0m 28s hadoop-aws in the patch passed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 67m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833118/HADOOP-13716-003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2cc204ff8944 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 9454dc5 Default Java 1.8.0_101 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10763/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          anu Anu Engineer added a comment -

          +1, thanks for doing this. It does improve the code reading experience tremendously, especially when using Java 8.

          One minor comment, In my mind fixed implies linear (Not a native english speaker), so I had to read the class comments to understand the subtle difference. Unfortunately, I am not able to come up with a better naming than what you have.

          Show
          anu Anu Engineer added a comment - +1, thanks for doing this. It does improve the code reading experience tremendously, especially when using Java 8. One minor comment, In my mind fixed implies linear (Not a native english speaker), so I had to read the class comments to understand the subtle difference. Unfortunately, I am not able to come up with a better naming than what you have.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I couldn't think of good names either, as the linear one is still less than any exponential backoff strategy.

          Show
          stevel@apache.org Steve Loughran added a comment - I couldn't think of good names either, as the linear one is still less than any exponential backoff strategy.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 004 for branch-2; stripped out the lambdas and ran the tests. Interesting that I had to make a local variable final...clearly java 8 is subtly different here

          hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java:[99,55] local variable root is accessed from within inner class; needs to be declared final
          

          Anu, thanks for +1-ing the patch. I want to do the checkstyle warnings on this before I apply it. This patch here doesn't do that: it's just patch 003 with everything compiling/running on branch-2

          Show
          stevel@apache.org Steve Loughran added a comment - patch 004 for branch-2; stripped out the lambdas and ran the tests. Interesting that I had to make a local variable final...clearly java 8 is subtly different here hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java:[99,55] local variable root is accessed from within inner class; needs to be declared final Anu, thanks for +1-ing the patch. I want to do the checkstyle warnings on this before I apply it. This patch here doesn't do that: it's just patch 003 with everything compiling/running on branch-2
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-13716 patch 005

          • rename methods to be more consistent with GTU.waitFor and scalatest
          • reorder params to put lambda expressions at end of methods
          • add detailed javadocs with examples, as this whole lambda-expression test stuff is new to the codebase
          • try to address checkstyle issues
          • move all java 8 test cases to same section of test suite
          • and rename everything to list entire set of closures
          • plus fix where java7 didn't compile AbstractContractRootDirectoryTest
          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-13716 patch 005 rename methods to be more consistent with GTU.waitFor and scalatest reorder params to put lambda expressions at end of methods add detailed javadocs with examples, as this whole lambda-expression test stuff is new to the codebase try to address checkstyle issues move all java 8 test cases to same section of test suite and rename everything to list entire set of closures plus fix where java7 didn't compile AbstractContractRootDirectoryTest
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 1m 5s Maven dependency ordering for branch
          +1 mvninstall 7m 16s trunk passed
          +1 compile 8m 9s trunk passed
          +1 checkstyle 1m 37s trunk passed
          +1 mvnsite 1m 32s trunk passed
          +1 mvneclipse 0m 38s trunk passed
          +1 findbugs 2m 17s trunk passed
          +1 javadoc 1m 13s trunk passed
          0 mvndep 0m 18s Maven dependency ordering for patch
          +1 mvninstall 1m 6s the patch passed
          +1 compile 8m 48s the patch passed
          -1 javac 8m 48s root generated 1 new + 701 unchanged - 1 fixed = 702 total (was 702)
          -0 checkstyle 1m 41s root: The patch generated 9 new + 25 unchanged - 0 fixed = 34 total (was 25)
          +1 mvnsite 1m 31s the patch passed
          +1 mvneclipse 0m 35s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 32s the patch passed
          +1 javadoc 1m 22s the patch passed
          +1 unit 8m 47s hadoop-common in the patch passed.
          +1 unit 0m 33s hadoop-aws in the patch passed.
          +1 asflicense 0m 31s The patch does not generate ASF License warnings.
          75m 10s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833351/HADOOP-13716-005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 962c60cb8391 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / dbe663d
          Default Java 1.8.0_101
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/console
          Powered by Apache Yetus 0.4.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 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 1m 5s Maven dependency ordering for branch +1 mvninstall 7m 16s trunk passed +1 compile 8m 9s trunk passed +1 checkstyle 1m 37s trunk passed +1 mvnsite 1m 32s trunk passed +1 mvneclipse 0m 38s trunk passed +1 findbugs 2m 17s trunk passed +1 javadoc 1m 13s trunk passed 0 mvndep 0m 18s Maven dependency ordering for patch +1 mvninstall 1m 6s the patch passed +1 compile 8m 48s the patch passed -1 javac 8m 48s root generated 1 new + 701 unchanged - 1 fixed = 702 total (was 702) -0 checkstyle 1m 41s root: The patch generated 9 new + 25 unchanged - 0 fixed = 34 total (was 25) +1 mvnsite 1m 31s the patch passed +1 mvneclipse 0m 35s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 32s the patch passed +1 javadoc 1m 22s the patch passed +1 unit 8m 47s hadoop-common in the patch passed. +1 unit 0m 33s hadoop-aws in the patch passed. +1 asflicense 0m 31s The patch does not generate ASF License warnings. 75m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833351/HADOOP-13716-005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 962c60cb8391 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dbe663d Default Java 1.8.0_101 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10788/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -
          1. I'll see about suppressing that javac warning
          2. checkstyle is being fussy about indentation in the lambda expressions. Not sure what to do there...maybe it's something the checker isn't ready for yet, or we need to look at its defaults.
          3. Anu: in the productions-sde retry code we have "ProportionalSleep"; I'll use that as the term here too
          Show
          stevel@apache.org Steve Loughran added a comment - I'll see about suppressing that javac warning checkstyle is being fussy about indentation in the lambda expressions. Not sure what to do there...maybe it's something the checker isn't ready for yet, or we need to look at its defaults. Anu: in the productions-sde retry code we have "ProportionalSleep"; I'll use that as the term here too
          Hide
          stevel@apache.org Steve Loughran added a comment -

          regarding the checkstyle indentations, it's saying things I don't agree with. Specifically:

              intercept(FailFastException.class,
                  () -> await(TIMEOUT,
                      () -> {
                        throw new FailFastException("ffe");   // HERE: should be 12 not 14
                      },                                                          // HERE: should be 10 not 12
                      retry,
                      (timeout, ex) -> ex));
          

          so: WONTFIX on those

          Show
          stevel@apache.org Steve Loughran added a comment - regarding the checkstyle indentations, it's saying things I don't agree with. Specifically: intercept(FailFastException.class, () -> await(TIMEOUT, () -> { throw new FailFastException( "ffe" ); // HERE: should be 12 not 14 }, // HERE: should be 10 not 12 retry, (timeout, ex) -> ex)); so: WONTFIX on those
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 006

          • switch to ProportionalIncreaseRetryInterval
          • suppress javac warning
          • move some more java7 test cases above the "java8 below" section in the test suite
          Show
          stevel@apache.org Steve Loughran added a comment - Patch 006 switch to ProportionalIncreaseRetryInterval suppress javac warning move some more java7 test cases above the "java8 below" section in the test suite
          Hide
          stevel@apache.org Steve Loughran added a comment -

          tested: S3a ireland

          Show
          stevel@apache.org Steve Loughran added a comment - tested: S3a ireland
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 1m 39s Maven dependency ordering for branch
          +1 mvninstall 6m 51s trunk passed
          +1 compile 6m 52s trunk passed
          +1 checkstyle 1m 27s trunk passed
          +1 mvnsite 1m 23s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 1m 49s trunk passed
          +1 javadoc 1m 0s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 0m 58s the patch passed
          +1 compile 6m 50s the patch passed
          +1 javac 6m 50s the patch passed
          -0 checkstyle 1m 29s root: The patch generated 9 new + 25 unchanged - 0 fixed = 34 total (was 25)
          +1 mvnsite 1m 25s the patch passed
          +1 mvneclipse 0m 36s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 12s the patch passed
          +1 javadoc 1m 7s the patch passed
          +1 unit 8m 15s hadoop-common in the patch passed.
          +1 unit 0m 28s hadoop-aws in the patch passed.
          +1 asflicense 0m 28s The patch does not generate ASF License warnings.
          69m 36s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833384/HADOOP-13716-006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 29c47a7d0307 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
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 391ce53
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/console
          Powered by Apache Yetus 0.4.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 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 1m 39s Maven dependency ordering for branch +1 mvninstall 6m 51s trunk passed +1 compile 6m 52s trunk passed +1 checkstyle 1m 27s trunk passed +1 mvnsite 1m 23s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 1m 49s trunk passed +1 javadoc 1m 0s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 0m 58s the patch passed +1 compile 6m 50s the patch passed +1 javac 6m 50s the patch passed -0 checkstyle 1m 29s root: The patch generated 9 new + 25 unchanged - 0 fixed = 34 total (was 25) +1 mvnsite 1m 25s the patch passed +1 mvneclipse 0m 36s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 12s the patch passed +1 javadoc 1m 7s the patch passed +1 unit 8m 15s hadoop-common in the patch passed. +1 unit 0m 28s hadoop-aws in the patch passed. +1 asflicense 0m 28s The patch does not generate ASF License warnings. 69m 36s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833384/HADOOP-13716-006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 29c47a7d0307 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 391ce53 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10803/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          [~any]: have you had a chance to look at this later patch?

          Show
          stevel@apache.org Steve Loughran added a comment - [~any] : have you had a chance to look at this later patch?
          Hide
          anu Anu Engineer added a comment -

          +1, I will commit this shortly to trunk, branch-2 and branch-2.8.

          A small nit :
          In LambdaTestUtils.java:84 a comment seems to be wrong.

              * Example: Wait 30s for a condition to be met, with a sleep of 30s
              

          I will fix this while committing to "sleep of 500ms" between each probe

          Show
          anu Anu Engineer added a comment - +1, I will commit this shortly to trunk, branch-2 and branch-2.8. A small nit : In LambdaTestUtils.java:84 a comment seems to be wrong. * Example: Wait 30s for a condition to be met, with a sleep of 30s I will fix this while committing to "sleep of 500ms" between each probe
          Hide
          anu Anu Engineer added a comment -

          Steve Loughran Thank you for the contribution. I have committed this to trunk, branch-2 and 2.8.

          Show
          anu Anu Engineer added a comment - Steve Loughran Thank you for the contribution. I have committed this to trunk, branch-2 and 2.8.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10647 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10647/)
          HADOOP-13716. Add LambdaTestUtils class for tests; fix eventual (aengineer: rev 3fbf4cd5da13dde68b77e581ea2d4aa564c8c8b7)

          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFailureHandling.java
          • (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java
          • (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/TestLambdaTestUtils.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
          • (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10647 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10647/ ) HADOOP-13716 . Add LambdaTestUtils class for tests; fix eventual (aengineer: rev 3fbf4cd5da13dde68b77e581ea2d4aa564c8c8b7) (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/ContractTestUtils.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFailureHandling.java (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/TestLambdaTestUtils.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java (edit) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
          Hide
          stevel@apache.org Steve Loughran added a comment -

          thanks for doing the commit lifting, though I'm going to have to tweak it...the branch-2 patch was out out of date,

          I'm going to leave trunk alone, revert branch 2 and reopen the JIRA until i can get round to things tomorrow morning.

          Show
          stevel@apache.org Steve Loughran added a comment - thanks for doing the commit lifting, though I'm going to have to tweak it...the branch-2 patch was out out of date, I'm going to leave trunk alone, revert branch 2 and reopen the JIRA until i can get round to things tomorrow morning.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 5s HADOOP-13716 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HADOOP-13716
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833384/HADOOP-13716-006.patch
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10869/console
          Powered by Apache Yetus 0.4.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 0s Docker mode activated. -1 patch 0m 5s HADOOP-13716 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-13716 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833384/HADOOP-13716-006.patch Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10869/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

            People

            • Assignee:
              stevel@apache.org Steve Loughran
              Reporter:
              stevel@apache.org Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development