Hadoop Common
  1. Hadoop Common
  2. HADOOP-6204

Implementing aspects development and fault injeciton framework for Hadoop

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: build, test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Fault injection framework implementation in HDFS (HDFS-435) turns out to be a very useful feature both for error handling testing and for various simulations.

      There's certain demand for this framework, thus it need to be pulled up from HDFS and brought into Common, so other sub-projects will be able to share it if needed.

      1. HADOOP-6204-ydist.patch
        22 kB
        Konstantin Boudnik
      2. HADOOP-6204-ydist.patch
        24 kB
        Konstantin Boudnik
      3. HADOOP-6204-ydist.patch
        24 kB
        Konstantin Boudnik
      4. HADOOP-6204-ydist.patch
        25 kB
        Konstantin Boudnik
      5. hadoop-6204-ydist.patch
        22 kB
        Sreekanth Ramakrishnan
      6. HADOOP-6204.patch.withmacros
        13 kB
        Konstantin Boudnik
      7. HADOOP-6204.patch.indirect
        13 kB
        Konstantin Boudnik
      8. HADOOP-6204.patch
        20 kB
        Konstantin Boudnik
      9. HADOOP-6204.patch
        27 kB
        Konstantin Boudnik
      10. HADOOP-6204.patch
        27 kB
        Konstantin Boudnik
      11. HADOOP-6204.patch
        28 kB
        Konstantin Boudnik
      12. HADOOP-6204.patch
        29 kB
        Konstantin Boudnik
      13. HADOOP-6204.patch
        30 kB
        Konstantin Boudnik
      14. HADOOP-6204.patch
        24 kB
        Konstantin Boudnik
      15. HADOOP-6204.patch
        24 kB
        Konstantin Boudnik
      16. HADOOP-6204.patch
        23 kB
        Konstantin Boudnik
      17. HADOOP-6204_0.20.patch
        22 kB
        Konstantin Boudnik
      18. HADOOP-6204_0.20.patch
        22 kB
        Konstantin Boudnik
      19. HADOOP-6204_0.20.patch
        22 kB
        Konstantin Boudnik
      20. HADOOP-6204_0.20.patch
        23 kB
        Konstantin Boudnik
      21. HADOOP-6204_0.20.patch
        23 kB
        Konstantin Boudnik
      22. HADOOP-6204_0.20.patch
        24 kB
        Konstantin Boudnik
      23. HADOOP-6204_0.20.patch
        24 kB
        Konstantin Boudnik

        Issue Links

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Konstantin Boudnik added a comment -

          To implement it the fault injection framework will have to be pulled from HDFS and include it into Common project. This can be eventually represented in a form of an auxiliary build file and ivy configuration, which could be used by other subprojects as SNV externals. A couple of framework classes would have to be migrated from HDFS to Common permanently.

          As the result, Common will need to provide one more jar file with instrumented classes, i.e. hadoop-core-0.21.0-dev-fi.jar

          Besides, it might be a good time to think of different name for this framework, because it is covering not only fault injection, but rather generic code instrumentation needs.

          Show
          Konstantin Boudnik added a comment - To implement it the fault injection framework will have to be pulled from HDFS and include it into Common project. This can be eventually represented in a form of an auxiliary build file and ivy configuration, which could be used by other subprojects as SNV externals. A couple of framework classes would have to be migrated from HDFS to Common permanently. As the result, Common will need to provide one more jar file with instrumented classes, i.e. hadoop-core-0.21.0-dev-fi.jar Besides, it might be a good time to think of different name for this framework, because it is covering not only fault injection, but rather generic code instrumentation needs.
          Hide
          Konstantin Boudnik added a comment -

          In order to avoid some massive ANT code duplication, I have sketched a solution for this as follows:

          • all FI logic is represented by a separate aop.xml file placed in build.aux folder
          • that aop.xml file is imported into build.xml
          • to make FI framework project independent I've renamed FI related targets to run-test-primary-fault-inject. run-test-secondary-fault-inject, jar-fault-inject, and jar-test-fault-inject
          • project specific build.properties should contains actual target to be executed by aforementioned generic names.

          Now, build.aux can be pulled from Common through svn:externals and then everything will be working in a very similar way. The above solution has 2 issues in my opinion:

          • an indirection level is being added. One way to avoid the abstraction is to define a number of macros in the aop.xml file and parametrize them from build.xml. It might be better approach.
          • svn:externals aren't universally acceptable. E.g. if one wants to pull SVN repository into a git one, then externals are have to be processed as a special case.

          I'd appreciate if anyone has any comment or preferences on this.

          Show
          Konstantin Boudnik added a comment - In order to avoid some massive ANT code duplication, I have sketched a solution for this as follows: all FI logic is represented by a separate aop.xml file placed in build.aux folder that aop.xml file is imported into build.xml to make FI framework project independent I've renamed FI related targets to run-test-primary-fault-inject . run-test-secondary-fault-inject , jar-fault-inject , and jar-test-fault-inject project specific build.properties should contains actual target to be executed by aforementioned generic names. Now, build.aux can be pulled from Common through svn:externals and then everything will be working in a very similar way. The above solution has 2 issues in my opinion: an indirection level is being added. One way to avoid the abstraction is to define a number of macros in the aop.xml file and parametrize them from build.xml. It might be better approach. svn:externals aren't universally acceptable. E.g. if one wants to pull SVN repository into a git one, then externals are have to be processed as a special case. I'd appreciate if anyone has any comment or preferences on this.
          Hide
          Konstantin Boudnik added a comment -

          This is the solution with macros in place and without additional indirection level for target names

          Show
          Konstantin Boudnik added a comment - This is the solution with macros in place and without additional indirection level for target names
          Hide
          Konstantin Boudnik added a comment -

          This is the solution with name resolutions (i.e. indirection layer)

          Both are created on top of existing HDFS' FI framework and will be adopted for Common in the final version of the patch.

          Show
          Konstantin Boudnik added a comment - This is the solution with name resolutions (i.e. indirection layer) Both are created on top of existing HDFS' FI framework and will be adopted for Common in the final version of the patch.
          Hide
          Konstantin Boudnik added a comment -

          This version represent pretty much final cut of the patch, which has only two minor issues. Namely, it fails to include fi-site.xml file into the -fi.jar file; and I need to find out why the package-info file is being put under normal build folder instead of build-fi one.

          The rest seems to be working properly, although more testing is needed.

          Show
          Konstantin Boudnik added a comment - This version represent pretty much final cut of the patch, which has only two minor issues. Namely, it fails to include fi-site.xml file into the -fi.jar file; and I need to find out why the package-info file is being put under normal build folder instead of build-fi one. The rest seems to be working properly, although more testing is needed.
          Hide
          Konstantin Boudnik added a comment -

          Everything seems to be in order now. The patch also provides needed classes for probability model and config manipulations.
          src/saveVersion.sh is fixed to generate package-info file under current build directory.

          Review is needed so the MapReduce and Hdfs will be able to reuse this very framework.

          Show
          Konstantin Boudnik added a comment - Everything seems to be in order now. The patch also provides needed classes for probability model and config manipulations. src/saveVersion.sh is fixed to generate package-info file under current build directory. Review is needed so the MapReduce and Hdfs will be able to reuse this very framework.
          Hide
          Konstantin Boudnik added a comment -

          Nicholas has raised the point if we need this modification in place by 0.21 release time.
          There are two main reason why I was pushing this into Common right now:

          • security tests: aspects framework will allow to test some AccessToken related functionality without permanent modifications of the production code. I'm not sure if this functionality will be included into 0.21 so I'd like to see any input on this
          • MR needs aspect framework in place for handling load simulations (MAPREDUCE-728). If this work doesn't require a permanent aspects framework in place and can live with a temp. fix then we can postpone this JIRA's commit past 0.21 release.

          Please comment.

          Show
          Konstantin Boudnik added a comment - Nicholas has raised the point if we need this modification in place by 0.21 release time. There are two main reason why I was pushing this into Common right now: security tests: aspects framework will allow to test some AccessToken related functionality without permanent modifications of the production code. I'm not sure if this functionality will be included into 0.21 so I'd like to see any input on this MR needs aspect framework in place for handling load simulations ( MAPREDUCE-728 ). If this work doesn't require a permanent aspects framework in place and can live with a temp. fix then we can postpone this JIRA's commit past 0.21 release. Please comment.
          Hide
          Konstantin Boudnik added a comment -

          Merged w/ trunk

          Show
          Konstantin Boudnik added a comment - Merged w/ trunk
          Hide
          Konstantin Boudnik added a comment -

          Now, src/test/aop is included into .eclipse.templates/.classpath

          Show
          Konstantin Boudnik added a comment - Now, src/test/aop is included into .eclipse.templates/.classpath
          Hide
          Konstantin Boudnik added a comment -

          Special run-fault-inject-with-testcaseonly target is added which allows to run normal test cases with instrumented code to reproduce possible regressions and such.

          This version of the patch also is better formatted compare to the last one and easy for review.

          Show
          Konstantin Boudnik added a comment - Special run-fault-inject-with-testcaseonly target is added which allows to run normal test cases with instrumented code to reproduce possible regressions and such. This version of the patch also is better formatted compare to the last one and easy for review.
          Hide
          Konstantin Boudnik added a comment -

          I'm running test-patch and getting this

          There appear to be 0 release audit warnings before the patch and 1 release audit warnings after applying the patch.
          
          
          
          
          -1 overall.
          
              +1 @author.  The patch does not contain any @author tags.
          
              +1 tests included.  The patch appears to include 20 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 warnings.
          
              -1 release audit.  The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings).
          
          

          However, that warning looks totally weird for me:

               [exec]   Copying the various non-generated resources to site.
               [exec]   Warnings will be issued if the optional project resources are not found.
               [exec]   This is often the case, because they are optional and so may not be available.
               [exec] Copying project resources and images to site ...
               [exec] Warning: /Users/xxx/work/Common.test/src/docs/build/webapp/resources not found.
               [exec] Copying main skin images to site ...
               [exec] Copying 18 files to /Users/xxx/work/Common.test/src/docs/build/site/skin/images
               [exec] Copying 14 files to /Users/xxx/work/Common.test/src/docs/build/site/skin/images
               [exec] Copying project skin images to site ...
               [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/common/images not found.
               [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/pelt/images not found.
               [exec] Copying main skin css and js files to site ...
               [exec] Copying 11 files to /Users/xxx/work/Common.test/src/docs/build/site/skin
               [exec] Copying 4 files to /Users/xxx/work/Common.test/src/docs/build/site/skin
               [exec] Copying project skin css and js files to site ...
               [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/common not found.
               [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/pelt not found.
          

          Apparently this patch has nothing to do with docs.

          Show
          Konstantin Boudnik added a comment - I'm running test-patch and getting this There appear to be 0 release audit warnings before the patch and 1 release audit warnings after applying the patch. -1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 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 warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). However, that warning looks totally weird for me: [exec] Copying the various non-generated resources to site. [exec] Warnings will be issued if the optional project resources are not found. [exec] This is often the case, because they are optional and so may not be available. [exec] Copying project resources and images to site ... [exec] Warning: /Users/xxx/work/Common.test/src/docs/build/webapp/resources not found. [exec] Copying main skin images to site ... [exec] Copying 18 files to /Users/xxx/work/Common.test/src/docs/build/site/skin/images [exec] Copying 14 files to /Users/xxx/work/Common.test/src/docs/build/site/skin/images [exec] Copying project skin images to site ... [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/common/images not found. [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/pelt/images not found. [exec] Copying main skin css and js files to site ... [exec] Copying 11 files to /Users/xxx/work/Common.test/src/docs/build/site/skin [exec] Copying 4 files to /Users/xxx/work/Common.test/src/docs/build/site/skin [exec] Copying project skin css and js files to site ... [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/common not found. [exec] Warning: /Users/xxx/work/Common.test/src/docs/src/documentation/skins/pelt not found. Apparently this patch has nothing to do with docs.
          Hide
          Konstantin Boudnik added a comment -

          This patch provides a common part of fault injection framework (src/test/aop/build, src/test/aop/org/apache/hadoop/fi/) and project specific modifications as a part of build.xml file.

          The generic part is suppose to be svn:exported to other subprojects (Hdfs, Mapreduce) and customized as needed.

          The patch includes some renameing in build.xml file i.e. core to java It has been done to make generic part of the FI build less project dependent. Similar modifications will be needed for Hdfs/MapReduce later on.

          A separate JIRA will be created to refactor currently existed FI framework in Hdfs to use the one from Common.

          Show
          Konstantin Boudnik added a comment - This patch provides a common part of fault injection framework (src/test/aop/build, src/test/aop/org/apache/hadoop/fi/) and project specific modifications as a part of build.xml file. The generic part is suppose to be svn:exported to other subprojects (Hdfs, Mapreduce) and customized as needed. The patch includes some renameing in build.xml file i.e. core to java It has been done to make generic part of the FI build less project dependent. Similar modifications will be needed for Hdfs/MapReduce later on. A separate JIRA will be created to refactor currently existed FI framework in Hdfs to use the one from Common.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12421703/HADOOP-6204.patch
          against trunk revision 823355.

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

          +1 tests included. The patch appears to include 20 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 warnings.

          -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings).

          -1 core tests. The patch failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/testReport/
          Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/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/12421703/HADOOP-6204.patch against trunk revision 823355. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 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 warnings. -1 release audit. The applied patch generated 1 release audit warnings (more than the trunk's current 0 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/76/console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          The cause of the release audit has been found. fi-site.xml was missing the standard Apache's boilerplate. My local test-patch log is attached:

          +1 overall.
          
              +1 @author.  The patch does not contain any @author tags.
          
              +1 tests included.  The patch appears to include 20 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 warnings.
          
              +1 release audit.  The applied patch does not increase the total number of release audit warnings.
          

          Also, there's no failing tests in the Hudson's run: all failures counters are zeroed. Looks like Hudson's going nuts.

          Show
          Konstantin Boudnik added a comment - The cause of the release audit has been found. fi-site.xml was missing the standard Apache's boilerplate. My local test-patch log is attached: +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 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 warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. Also, there's no failing tests in the Hudson's run: all failures counters are zeroed. Looks like Hudson's going nuts.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > Also, there's no failing tests in the Hudson's run: all failures counters are zeroed. Looks like Hudson's going nuts.

          Checked the raw log. Look like that TestDNS did not success.

               [exec]     [junit] Running org.apache.hadoop.net.TestDNS
               [exec]     [junit] 2009-10-09 05:55:16,217 INFO  net.TestDNS (TestDNS.java:testRDNS(130)) - Reverse DNS failing as due to incomplete networking
               [exec]     [junit] javax.naming.NameNotFoundException: DNS name not found [response code 3]; remaining name '1.1.0.127.in-addr.arpa'
               [exec]     [junit] 	at com.sun.jndi.dns.DnsClient.checkResponseCode(DnsClient.java:596)
               [exec]     [junit] 	at com.sun.jndi.dns.DnsClient.isMatchResponse(DnsClient.java:553)
               [exec]     [junit] 	at com.sun.jndi.dns.DnsClient.doUdpQuery(DnsClient.java:399)
               [exec]     [junit] 	at com.sun.jndi.dns.DnsClient.query(DnsClient.java:186)
               [exec]     [junit] 	at com.sun.jndi.dns.Resolver.query(Resolver.java:64)
               [exec]     [junit] 	at com.sun.jndi.dns.DnsContext.c_getAttributes(DnsContext.java:413)
               [exec]     [junit] 	at com.sun.jndi.toolkit.ctx.ComponentDirContext.p_getAttributes(ComponentDirContext.java:213)
               [exec]     [junit] 	at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.getAttributes(PartialCompositeDirContext.java:121)
               [exec]     [junit] 	at com.sun.jndi.toolkit.url.GenericURLDirContext.getAttributes(GenericURLDirContext.java:85)
               [exec]     [junit] 	at javax.naming.directory.InitialDirContext.getAttributes(InitialDirContext.java:123)
               [exec]     [junit] 	at org.apache.hadoop.net.DNS.reverseDns(DNS.java:78)
               [exec]     [junit] 	at org.apache.hadoop.net.TestDNS.testRDNS(TestDNS.java:124)
               [exec]     [junit] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
               [exec]     [junit] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
               [exec]     [junit] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
               [exec]     [junit] 	at java.lang.reflect.Method.invoke(Method.java:597)
               [exec]     [junit] 	at junit.framework.TestCase.runTest(TestCase.java:168)
               [exec]     ...
          
          Show
          Tsz Wo Nicholas Sze added a comment - > Also, there's no failing tests in the Hudson's run: all failures counters are zeroed. Looks like Hudson's going nuts. Checked the raw log. Look like that TestDNS did not success. [exec] [junit] Running org.apache.hadoop.net.TestDNS [exec] [junit] 2009-10-09 05:55:16,217 INFO net.TestDNS (TestDNS.java:testRDNS(130)) - Reverse DNS failing as due to incomplete networking [exec] [junit] javax.naming.NameNotFoundException: DNS name not found [response code 3]; remaining name '1.1.0.127.in-addr.arpa' [exec] [junit] at com.sun.jndi.dns.DnsClient.checkResponseCode(DnsClient.java:596) [exec] [junit] at com.sun.jndi.dns.DnsClient.isMatchResponse(DnsClient.java:553) [exec] [junit] at com.sun.jndi.dns.DnsClient.doUdpQuery(DnsClient.java:399) [exec] [junit] at com.sun.jndi.dns.DnsClient.query(DnsClient.java:186) [exec] [junit] at com.sun.jndi.dns.Resolver.query(Resolver.java:64) [exec] [junit] at com.sun.jndi.dns.DnsContext.c_getAttributes(DnsContext.java:413) [exec] [junit] at com.sun.jndi.toolkit.ctx.ComponentDirContext.p_getAttributes(ComponentDirContext.java:213) [exec] [junit] at com.sun.jndi.toolkit.ctx.PartialCompositeDirContext.getAttributes(PartialCompositeDirContext.java:121) [exec] [junit] at com.sun.jndi.toolkit.url.GenericURLDirContext.getAttributes(GenericURLDirContext.java:85) [exec] [junit] at javax.naming.directory.InitialDirContext.getAttributes(InitialDirContext.java:123) [exec] [junit] at org.apache.hadoop.net.DNS.reverseDns(DNS.java:78) [exec] [junit] at org.apache.hadoop.net.TestDNS.testRDNS(TestDNS.java:124) [exec] [junit] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) [exec] [junit] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) [exec] [junit] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) [exec] [junit] at java.lang.reflect.Method.invoke(Method.java:597) [exec] [junit] at junit.framework.TestCase.runTest(TestCase.java:168) [exec] ...
          Hide
          Konstantin Boudnik added a comment -
          • Checked the raw log. Look like that TestDNS did not success.

            Right, which doesn't look related to the patch. I'll re-run everything manually just to make sure.

          • from Hudson's test results page:
            Test Result 0 failures (±0)
            585 tests (±0) Took 4 min 1 sec.
            

            Which confirms that Hudson went nuts

          Show
          Konstantin Boudnik added a comment - Checked the raw log. Look like that TestDNS did not success. Right, which doesn't look related to the patch. I'll re-run everything manually just to make sure. from Hudson's test results page: Test Result 0 failures (±0) 585 tests (±0) Took 4 min 1 sec. Which confirms that Hudson went nuts
          Hide
          Konstantin Boudnik added a comment -

          As per Jacob's suggestions I'll split this patch in two: this one and HADOOP-6305. New patch to be posted shortly

          Show
          Konstantin Boudnik added a comment - As per Jacob's suggestions I'll split this patch in two: this one and HADOOP-6305 . New patch to be posted shortly
          Hide
          Konstantin Boudnik added a comment -

          I've run all fault injection related targets, also have done an injection into the code and ran tests successfully.

          There appear to be 0 release audit warnings before the patch and 0 release audit warnings after applying the patch.
          
          +1 overall.
              +1 @author.  The patch does not contain any @author tags.
              +1 tests included.  The patch appears to include 19 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 warnings.
              +1 release audit.  The applied patch does not increase the total number of release audit warnings.
          
          ======================================================================
          ======================================================================
              Finished build.
          
          Show
          Konstantin Boudnik added a comment - I've run all fault injection related targets, also have done an injection into the code and ran tests successfully. There appear to be 0 release audit warnings before the patch and 0 release audit warnings after applying the patch. +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 19 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 warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. ====================================================================== ====================================================================== Finished build.
          Hide
          Konstantin Boudnik added a comment -

          Resubmitting patch after it has been split between this and HADOOP-6305

          Show
          Konstantin Boudnik added a comment - Resubmitting patch after it has been split between this and HADOOP-6305
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12421795/HADOOP-6204.patch
          against trunk revision 823756.

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

          +1 tests included. The patch appears to include 19 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 warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/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/12421795/HADOOP-6204.patch against trunk revision 823756. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 19 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 warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/79/console This message is automatically generated.
          Hide
          Konstantin Boudnik added a comment -

          I can see same issue with TestDNS as has been previously mentioned by Jacob, however, the Hudson's test report doesn't show any failures in it.
          So, looks like the patch is cool cause that supposed test failure isn't related to it.

          Show
          Konstantin Boudnik added a comment - I can see same issue with TestDNS as has been previously mentioned by Jacob, however, the Hudson's test report doesn't show any failures in it. So, looks like the patch is cool cause that supposed test failure isn't related to it.
          Hide
          Konstantin Boudnik added a comment -

          All attributes of a macro have to be initialized at invocation. Fixed

          Show
          Konstantin Boudnik added a comment - All attributes of a macro have to be initialized at invocation. Fixed
          Hide
          Jakob Homan added a comment -

          Patch review based on 10/12 patch:

          Looks good overall with just a couple of things.

          • Line 247: The TODO comment needs to be removed. The method seems appropriately sophisticated, but if it's to be improved that should be done either now or in a separate JIRA, but leaving the comment in the code won't work.
          • Line 273: If may be useful to log if an invalid property has been specified in the config file.
          • The new target injectfaults doesn't follow the convention of separating words with a hyphen.

          Nits:

          • Lines 132 and 151, what's the point of the empty space within the comments?
          • Line 321-326, is it necessary to have init be a separate method rather than inlining into the static block from which it is called I'm not seeing the purpose of the separate method.
          Show
          Jakob Homan added a comment - Patch review based on 10/12 patch: Looks good overall with just a couple of things. Line 247: The TODO comment needs to be removed. The method seems appropriately sophisticated, but if it's to be improved that should be done either now or in a separate JIRA, but leaving the comment in the code won't work. Line 273: If may be useful to log if an invalid property has been specified in the config file. The new target injectfaults doesn't follow the convention of separating words with a hyphen. Nits: Lines 132 and 151, what's the point of the empty space within the comments? Line 321-326, is it necessary to have init be a separate method rather than inlining into the static block from which it is called I'm not seeing the purpose of the separate method.
          Hide
          Konstantin Boudnik added a comment -

          Jacob's comments are addressed except this one:

          * The new target injectfaults doesn't follow the convention of separating words with a hyphen.

          This target won't be called by anyone directly except a very few developers working on faults and aspects. Thus, the name reflects it by no following the convention for the rest of the target.

          Show
          Konstantin Boudnik added a comment - Jacob's comments are addressed except this one: * The new target injectfaults doesn't follow the convention of separating words with a hyphen. This target won't be called by anyone directly except a very few developers working on faults and aspects. Thus, the name reflects it by no following the convention for the rest of the target.
          Hide
          Jakob Homan added a comment -

          That's a reasonable explanation. +1 on latest version of patch.

          Show
          Jakob Homan added a comment - That's a reasonable explanation. +1 on latest version of patch.
          Hide
          Konstantin Boudnik added a comment -

          I've just committed this.

          Show
          Konstantin Boudnik added a comment - I've just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #61 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/61/)
          . Implementing aspects development and fault injeciton framework for Hadoop. Contributed by Konstantin Boudnik

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #61 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/61/ ) . Implementing aspects development and fault injeciton framework for Hadoop. Contributed by Konstantin Boudnik
          Hide
          Tsz Wo Nicholas Sze added a comment -

          This was also committed to 0.21.

          Show
          Tsz Wo Nicholas Sze added a comment - This was also committed to 0.21.
          Hide
          Sreekanth Ramakrishnan added a comment -

          Attaching Yahoo! Distribution patch.

          Show
          Sreekanth Ramakrishnan added a comment - Attaching Yahoo! Distribution patch.
          Hide
          Konstantin Boudnik added a comment -

          Clearly, the latest patch is for 0.20 source tree.

          Show
          Konstantin Boudnik added a comment - Clearly, the latest patch is for 0.20 source tree.
          Hide
          Konstantin Boudnik added a comment -

          AspectJ version is set to 1.6.5. Possible OOM problem is fixed. Some blank lines modifications are removed.

          Show
          Konstantin Boudnik added a comment - AspectJ version is set to 1.6.5. Possible OOM problem is fixed. Some blank lines modifications are removed.
          Hide
          Konstantin Boudnik added a comment -

          Updated version of the patch for 0.20: adds fault-injected tests to the main test target; fixes an additional empty line mod.

          Show
          Konstantin Boudnik added a comment - Updated version of the patch for 0.20: adds fault-injected tests to the main test target; fixes an additional empty line mod.
          Hide
          Konstantin Boudnik added a comment -

          Latest patch for 0.20 branch.
          The way junit task is configured is to look for all tests under src/test directory. It will include and try to run tests in src/test/aop folder during execution of normal test-core target. Such attempt will clearly fail. This patch fixes this issue.

          Show
          Konstantin Boudnik added a comment - Latest patch for 0.20 branch. The way junit task is configured is to look for all tests under src/test directory. It will include and try to run tests in src/test/aop folder during execution of normal test-core target. Such attempt will clearly fail. This patch fixes this issue.
          Hide
          Konstantin Boudnik added a comment -

          This patch in combination with latest ydist_20 reflects the patch for 0.20 branch

          Show
          Konstantin Boudnik added a comment - This patch in combination with latest ydist_20 reflects the patch for 0.20 branch
          Hide
          Konstantin Boudnik added a comment -

          Excluding AOP directories from test lookup process. Previous attempt to limit the lookup to ${test.src.dir/org}} failed

          Show
          Konstantin Boudnik added a comment - Excluding AOP directories from test lookup process. Previous attempt to limit the lookup to ${test.src.dir /org}} failed
          Hide
          Konstantin Boudnik added a comment -

          This patch includes the update for missing jar files in eclipse .classpath file.
          The results of local test-patch verification are below:

          +1 overall.
          
              +1 @author.  The patch does not contain any @author tags.
          
              +1 tests included.  The patch appears to include 18 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 warnings.
          
              +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          
          Show
          Konstantin Boudnik added a comment - This patch includes the update for missing jar files in eclipse .classpath file. The results of local test-patch verification are below: +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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 warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          Hide
          Konstantin Boudnik added a comment -

          Also, if this JIRA will ever be committed to the branch 0.20 then documentation (as per HDFS-498) needs to be backported as well.

          Show
          Konstantin Boudnik added a comment - Also, if this JIRA will ever be committed to the branch 0.20 then documentation (as per HDFS-498 ) needs to be backported as well.
          Hide
          Konstantin Boudnik added a comment -

          Mis-bound warnings have to be interpreted as errors.

          Show
          Konstantin Boudnik added a comment - Mis-bound warnings have to be interpreted as errors.
          Hide
          Konstantin Boudnik added a comment -

          This latest version of the patch includes a feature which has been missed from the original patch and has been later addressed by a separate JIRA (HDFS-584).
          Simply put, a build has to be fail if some aspects are mis-bound.

          Show
          Konstantin Boudnik added a comment - This latest version of the patch includes a feature which has been missed from the original patch and has been later addressed by a separate JIRA ( HDFS-584 ). Simply put, a build has to be fail if some aspects are mis-bound.
          Hide
          Konstantin Boudnik added a comment -

          Yet another glitch is found in the initial backport patch. This version fixes incorrect name of FI-test jar file (for 0.20 branch).

          Show
          Konstantin Boudnik added a comment - Yet another glitch is found in the initial backport patch. This version fixes incorrect name of FI-test jar file (for 0.20 branch).
          Hide
          Konstantin Boudnik added a comment -

          Instead of tracking small fixes in the separate sub-tasks I've prepared this patch which correlates with branch-0.20 patch and will be committed to ydist.

          Show
          Konstantin Boudnik added a comment - Instead of tracking small fixes in the separate sub-tasks I've prepared this patch which correlates with branch-0.20 patch and will be committed to ydist.
          Hide
          Konstantin Boudnik added a comment -

          And yet another issue is found

          Show
          Konstantin Boudnik added a comment - And yet another issue is found
          Hide
          Konstantin Boudnik added a comment -

          Seems to be a final version for ydist fixing the invocation of call of non-existing target 'run-test-core'

          Show
          Konstantin Boudnik added a comment - Seems to be a final version for ydist fixing the invocation of call of non-existing target 'run-test-core'
          Hide
          Konstantin Boudnik added a comment -

          Local run of test-patch for the latest version of 0.20 branch's patch

          +1 overall.
          
              +1 @author.  The patch does not contain any @author tags.
          
              +1 tests included.  The patch appears to include 18 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 warnings.
          
              +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
          
          Show
          Konstantin Boudnik added a comment - Local run of test-patch for the latest version of 0.20 branch's patch +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 18 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 warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

            People

            • Assignee:
              Konstantin Boudnik
              Reporter:
              Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development