Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-584

Fail the fault-inject build if any advices are mis-bound

    Details

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

      Description

      Whenever AspectJ compiler can't bind an advice it issues a warning message, but the build process isn't failing because of it.
      The build has to fail though, because mis-bound advices lead to failing tests which are hard to explain otherwise.

      1. HDFS-584.patch
        2 kB
        Konstantin Boudnik
      2. HDFS-584.patch
        2 kB
        Konstantin Boudnik
      3. HDFS-584.patch
        2 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Hide
          Konstantin Boudnik added a comment -

          It turns out that mis-bound warnings couldn't be simply turned into errors, because this is default behavior of AJC and I don't see a way to configure it differently.

          The only possibility is to intercept the compiler output and parse it in order to find if any warnings were produced and abort the build in that case. But it seems to be too much trouble to do so.

          Another possibility is to extend test-patch so it will verify that a new patch doesn't increase the number of AJC warnings like it currently does for javac.

          Show
          Konstantin Boudnik added a comment - It turns out that mis-bound warnings couldn't be simply turned into errors, because this is default behavior of AJC and I don't see a way to configure it differently. The only possibility is to intercept the compiler output and parse it in order to find if any warnings were produced and abort the build in that case. But it seems to be too much trouble to do so. Another possibility is to extend test-patch so it will verify that a new patch doesn't increase the number of AJC warnings like it currently does for javac.
          Hide
          Konstantin Boudnik added a comment -

          After all it seems to be possible to fix it my parsing AspectJ compiler output

          Show
          Konstantin Boudnik added a comment - After all it seems to be possible to fix it my parsing AspectJ compiler output
          Hide
          Konstantin Boudnik added a comment -

          Patch allows to copy injectfaults target output to a separate file for future analysis.
          If AJC warnings are found the build failed

          Show
          Konstantin Boudnik added a comment - Patch allows to copy injectfaults target output to a separate file for future analysis. If AJC warnings are found the build failed
          Hide
          Konstantin Boudnik added a comment -

          I ran the build on healthy code and it went through as usual.

          Then I've intentionally broke some advice bindings and started the build

            % ant injectfaults
          

          and got these effect:

          compile-fault-inject:
               [echo] Start weaving aspects in place
               [iajc] warning at /Users/cos/work/Hdfs.inj/src/test/aop/org/apache/hadoop/hdfs/server/datanode/FSDatasetAspects.aj:55::0 advice defined in org.apache.hadoop.hdfs.server.datanode.FSDatasetAspects has not been a
          pplied [Xlint:adviceDidNotMatch]
          
          BUILD FAILED
          /Users/cos/work/Hdfs.inj/build.xml:346: The following error occurred while executing this line:
          /Users/cos/work/Hdfs.inj/build.xml:338: Broken binding of advices:
               [iajc] warning at /Users/cos/work/Hdfs.inj/src/test/aop/org/apache/hadoop/hdfs/server/datanode/FSDatasetAspects.aj:55::0 advice defined in org.apache.hadoop.hdfs.server.datanode.FSDatasetAspects has not been a
          pplied [Xlint:adviceDidNotMatch]
          
          Total time: 37 seconds
          
          Show
          Konstantin Boudnik added a comment - I ran the build on healthy code and it went through as usual. Then I've intentionally broke some advice bindings and started the build % ant injectfaults and got these effect: compile-fault-inject: [echo] Start weaving aspects in place [iajc] warning at /Users/cos/work/Hdfs.inj/src/test/aop/org/apache/hadoop/hdfs/server/datanode/FSDatasetAspects.aj:55::0 advice defined in org.apache.hadoop.hdfs.server.datanode.FSDatasetAspects has not been a pplied [Xlint:adviceDidNotMatch] BUILD FAILED /Users/cos/work/Hdfs.inj/build.xml:346: The following error occurred while executing this line: /Users/cos/work/Hdfs.inj/build.xml:338: Broken binding of advices: [iajc] warning at /Users/cos/work/Hdfs.inj/src/test/aop/org/apache/hadoop/hdfs/server/datanode/FSDatasetAspects.aj:55::0 advice defined in org.apache.hadoop.hdfs.server.datanode.FSDatasetAspects has not been a pplied [Xlint:adviceDidNotMatch] Total time: 37 seconds
          Hide
          Konstantin Boudnik added a comment -

          This patch effectively copies the HADOOP-6326 until HDFS-703 isn't implemented.

          Show
          Konstantin Boudnik added a comment - This patch effectively copies the HADOOP-6326 until HDFS-703 isn't implemented.
          Hide
          Konstantin Boudnik added a comment -

          Fixes wrong property name

          Show
          Konstantin Boudnik added a comment - Fixes wrong property name
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Need to check whether the file exists before deleting it. Otherwise, the task will fail.

          +    <delete file="${compile-inject.output}"/>
          
          Show
          Tsz Wo Nicholas Sze added a comment - Need to check whether the file exists before deleting it. Otherwise, the task will fail. + <delete file= "${compile-inject.output}" />
          Hide
          Konstantin Boudnik added a comment -

          Actually, it won't:

            % rm build-fi/compile-fi.log && ant injectfaults
          Buildfile: build.xml
          
          injectfaults:
          ...
          
          compile-fault-inject:
               [echo] Start weaving aspects in place
               [echo] Weaving of aspects is finished
          
          BUILD SUCCESSFUL
          Total time: 42 seconds
          
          Show
          Konstantin Boudnik added a comment - Actually, it won't: % rm build-fi/compile-fi.log && ant injectfaults Buildfile: build.xml injectfaults: ... compile-fault-inject: [echo] Start weaving aspects in place [echo] Weaving of aspects is finished BUILD SUCCESSFUL Total time: 42 seconds
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Tried "ant clean test -Dtestcase=..."

          compile-fault-inject:
               [echo] Start weaving aspects in place
          
          BUILD FAILED
          /home/tsz/hadoop/hdfs/h1/build.xml:630: The following error occurred while executing this line:
          /home/tsz/hadoop/hdfs/h1/build.xml:348: The following error occurred while executing this line:
          /home/tsz/hadoop/hdfs/h1/build.xml:331: /home/tsz/hadoop/hdfs/h1/build-fi/compile-fi.log doesn't exist
          
          Total time: 48 seconds
          
          Show
          Tsz Wo Nicholas Sze added a comment - Tried "ant clean test -Dtestcase=..." compile-fault-inject: [echo] Start weaving aspects in place BUILD FAILED /home/tsz/hadoop/hdfs/h1/build.xml:630: The following error occurred while executing this line: /home/tsz/hadoop/hdfs/h1/build.xml:348: The following error occurred while executing this line: /home/tsz/hadoop/hdfs/h1/build.xml:331: /home/tsz/hadoop/hdfs/h1/build-fi/compile-fi.log doesn't exist Total time: 48 seconds
          Hide
          Tsz Wo Nicholas Sze added a comment -

          We probably should check various ant targets for this change.

          Show
          Tsz Wo Nicholas Sze added a comment - We probably should check various ant targets for this change.
          Hide
          Konstantin Boudnik added a comment -

          Thanks for catching this, Nicholas!

          It isn't a delete problem though. It happens because subant is trying to set output while init target hasn't ran for injectfaults yet. Thus, build-fi folder hasn't been created yet.
          Delete, on the other hand doesn't care about missing file. It simply states this fact as in {{ [delete] Could not find file /Users/cos/work/Common/build-fi/compile-fi.log to delete.}}

          This new version of the patch takes care about this case.

          Show
          Konstantin Boudnik added a comment - Thanks for catching this, Nicholas! It isn't a delete problem though. It happens because subant is trying to set output while init target hasn't ran for injectfaults yet. Thus, build-fi folder hasn't been created yet. Delete, on the other hand doesn't care about missing file. It simply states this fact as in {{ [delete] Could not find file /Users/cos/work/Common/build-fi/compile-fi.log to delete.}} This new version of the patch takes care about this case.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Hide
          Konstantin Boudnik added a comment -

          There's no need for full test-patch verification process because the effect of this fix appears only when an advice binding is broken.

          Thanks for the review, Nicholas.

          I've just committed this.

          Show
          Konstantin Boudnik added a comment - There's no need for full test-patch verification process because the effect of this fix appears only when an advice binding is broken. Thanks for the review, Nicholas. I've just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/82/)
          . Fail the fault-inject build if any advices are mis-bound. Contributed by Konstantin Boudnik

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/82/ ) . Fail the fault-inject build if any advices are mis-bound. Contributed by Konstantin Boudnik
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #59 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/59/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #59 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/59/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #120 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/120/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #120 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/120/ )
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/ )

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development