Hadoop Common
  1. Hadoop Common
  2. HADOOP-6788

[Herriot] Exception exclusion functionality is not working correctly.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      herriot

      Description

      Exception exclusion functionality is not working correctly because of that tests are failing by not matching the error count.
      I debugged the issue and found that the problem with shell command which is generating in the getNumberOfMatchesInLogFile function.

      Currently building the shell command in the following way.

      if(list != null){
      for(int i =0; i < list.length; ++i)

      { filePattern.append(" | grep -v " + list[i] ); }

      }
      String[] cmd =
      new String[] {
      "bash",
      "-c",
      "grep -c "
      + pattern + " " + filePattern
      + " | awk -F: '

      {s+=$2} END {print s}'" };

      However, The above commnad won't work correctly because you are counting the exceptions in the file before excluding the known exceptions.
      In this case it gives the mismatch error counts everytime.The shell command should be in the following way to work correctly.

      if (list != null) {
      int index = 0;
      for (String excludeExp : list) { filePattern.append((++index < list.length)? "| grep -v " : "| grep -vc " + list[i] ); }
      }
      String[] cmd =
      new String[] {
      "bash",
      "-c",
      "grep "
      + pattern + " " + filePattern
      + " | awk -F: '{s+=$2}

      END

      {print s}

      '" };

      1. 6788-ydist-security.patch
        2 kB
        Vinay Kumar Thota
      2. 6788-ydist-security.patch
        2 kB
        Vinay Kumar Thota
      3. HADOOP-6788.patch
        2 kB
        Vinay Kumar Thota
      4. HADOOP-6788.patch
        2 kB
        Vinay Kumar Thota

        Issue Links

          Activity

          Hide
          Konstantin Boudnik added a comment -

          Framework is needed.

          Show
          Konstantin Boudnik added a comment - Framework is needed.
          Hide
          Konstantin Boudnik added a comment -

          It isn't a good style to paste a content of the patch into the description field.

          Show
          Konstantin Boudnik added a comment - It isn't a good style to paste a content of the patch into the description field.
          Hide
          Konstantin Boudnik added a comment -

          The above suggestion seems to be faulty because it recommends to add the exclusions to the filePattern instead of pattern. Essentially, grep will be looking for pattern inside of the files named after exceptions intended to be excluded.

          Show
          Konstantin Boudnik added a comment - The above suggestion seems to be faulty because it recommends to add the exclusions to the filePattern instead of pattern . Essentially, grep will be looking for pattern inside of the files named after exceptions intended to be excluded.
          Hide
          Vinay Kumar Thota added a comment -

          It's my suggestion where exactly the code needs to change to resolve the issue.So that I have mentioned the part of the code in description field.

          The pattern always should be either ERROR,WARN and FATAL and we need to fetch the exceptions based on the pattern from the file.Once we got the exceptions, we need to exclude the exceptions from the output list.Later we need to count the new exceptions.

          For example, In my above suggestion the shell command generates like this.

          grap ERROR logfiles* | grep -v IOExceptin | grep -vc java.net.ConnectException | awk -F : '

          {s+=$2}

          END

          {print s}

          '

          here filePattern is logfile* and pattern is ERROR

          I would say my suggestion is 100% correct and there is no faulty in that.

          Show
          Vinay Kumar Thota added a comment - It's my suggestion where exactly the code needs to change to resolve the issue.So that I have mentioned the part of the code in description field. The pattern always should be either ERROR,WARN and FATAL and we need to fetch the exceptions based on the pattern from the file.Once we got the exceptions, we need to exclude the exceptions from the output list.Later we need to count the new exceptions. For example, In my above suggestion the shell command generates like this. grap ERROR logfiles* | grep -v IOExceptin | grep -vc java.net.ConnectException | awk -F : ' {s+=$2} END {print s} ' here filePattern is logfile* and pattern is ERROR I would say my suggestion is 100% correct and there is no faulty in that.
          Hide
          Konstantin Boudnik added a comment -

          oops, you are right. I have misread the proposed fix. Sorry. I guess this is the absence of a patch to blame

          Show
          Konstantin Boudnik added a comment - oops, you are right. I have misread the proposed fix. Sorry. I guess this is the absence of a patch to blame
          Hide
          Konstantin Boudnik added a comment -

          Looks like the patch has been lost in transition? Is that right?

          Show
          Konstantin Boudnik added a comment - Looks like the patch has been lost in transition? Is that right?
          Hide
          Vinay Kumar Thota added a comment -

          Yes, earlier I have opened this issue under Mapreduce specific and now I Changed to common.

          -Vinay

          Show
          Vinay Kumar Thota added a comment - Yes, earlier I have opened this issue under Mapreduce specific and now I Changed to common. -Vinay
          Hide
          Vinay Kumar Thota added a comment -

          Fixed the issue and patch attached for yahoo distribution security branch.

          Show
          Vinay Kumar Thota added a comment - Fixed the issue and patch attached for yahoo distribution security branch.
          Hide
          Vinay Kumar Thota added a comment -

          Patch for common trunk

          Show
          Vinay Kumar Thota added a comment - Patch for common trunk
          Hide
          Balaji Rajagopalan added a comment -

          Looks good to me.

          Show
          Balaji Rajagopalan added a comment - Looks good to me.
          Hide
          Konstantin Boudnik added a comment -

          This looks like a whitespace modification to me:

          -                + " | awk -F: '{s+=$2} END {print s}'" };    
          +                + " | awk -F: '{s+=$2} END {print s}'" };
          

          Please fix it. Other than that looks ok.

          Show
          Konstantin Boudnik added a comment - This looks like a whitespace modification to me: - + " | awk -F: '{s+=$2} END {print s}'" }; + + " | awk -F: '{s+=$2} END {print s}'" }; Please fix it. Other than that looks ok.
          Hide
          Vinay Kumar Thota added a comment -

          Cos,
          Yes, you are correct. Earlier there was four white spaces after that statement and I have removed them while modifying code,because I felt that its unnecessary to keep the white spaces after the statement. I just curious to know, is there a necessity to create a another patch for this? I feels that its not an issue right.

          Show
          Vinay Kumar Thota added a comment - Cos, Yes, you are correct. Earlier there was four white spaces after that statement and I have removed them while modifying code,because I felt that its unnecessary to keep the white spaces after the statement. I just curious to know, is there a necessity to create a another patch for this? I feels that its not an issue right.
          Hide
          Vinay Kumar Thota added a comment -

          Attached new patch based on cos comments. Patch for yahoo security distribution branch.

          Show
          Vinay Kumar Thota added a comment - Attached new patch based on cos comments. Patch for yahoo security distribution branch.
          Hide
          Vinay Kumar Thota added a comment -

          New patch for trunk.

          Show
          Vinay Kumar Thota added a comment - New patch for trunk.
          Hide
          Konstantin Boudnik added a comment -

          Vinay, code re-formats should be done separately from code modifications unless they are a part of some fixes. The reason is simple: formatting in the patches makes reviewing much harder, especially for bigger patches.

          I have applied the patch to the Common's code, built the artifacts and then re-built HDFS using fresh instrumented Common. All code seem to be in place.

          +1 patch looks good.

          Show
          Konstantin Boudnik added a comment - Vinay, code re-formats should be done separately from code modifications unless they are a part of some fixes. The reason is simple: formatting in the patches makes reviewing much harder, especially for bigger patches. I have applied the patch to the Common's code, built the artifacts and then re-built HDFS using fresh instrumented Common. All code seem to be in place. +1 patch looks good.
          Hide
          Konstantin Boudnik added a comment -

          To make sure that everything is all right.

          Show
          Konstantin Boudnik added a comment - To make sure that everything is all right.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Can you please augment the title of all future herriot related issues with [Herriot]? Helps other people not concerned, avoids confusion, so... Thanks!

          Show
          Vinod Kumar Vavilapalli added a comment - Can you please augment the title of all future herriot related issues with [Herriot] ? Helps other people not concerned, avoids confusion, so... Thanks!
          Hide
          Konstantin Boudnik added a comment -

          Can you please augment the title of all future herriot related issues with [Herriot]?

          Well, I'm not sure about this. Herriot never was approved/discussed by the community as an official name of the framework. Let's think about this first.

          Show
          Konstantin Boudnik added a comment - Can you please augment the title of all future herriot related issues with [Herriot] ? Well, I'm not sure about this. Herriot never was approved/discussed by the community as an official name of the framework. Let's think about this first.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12446103/HADOOP-6788.patch
          against trunk revision 951081.

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

          +1 tests included. The patch appears to include 3 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 passed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/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/12446103/HADOOP-6788.patch against trunk revision 951081. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/74/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12446103/HADOOP-6788.patch
          against trunk revision 951081.

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

          +1 tests included. The patch appears to include 3 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 passed 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/559/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/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/12446103/HADOOP-6788.patch against trunk revision 951081. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 passed 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/559/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/559/console This message is automatically generated.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Well, I'm not sure about this. Herriot never was approved/discussed by the community as an official name of the framework. Let's think about this first.

          Sure Kos. My main concern is that the titles for most of these issues are fairly generic and on a surface look aren't very reflective of what they are really about, and which component.

          So, yes, I'll stop messing with the titles till the name issue is resolved.

          Show
          Vinod Kumar Vavilapalli added a comment - Well, I'm not sure about this. Herriot never was approved/discussed by the community as an official name of the framework. Let's think about this first. Sure Kos. My main concern is that the titles for most of these issues are fairly generic and on a surface look aren't very reflective of what they are really about, and which component. So, yes, I'll stop messing with the titles till the name issue is resolved.
          Hide
          Konstantin Boudnik added a comment -

          Well, first we need to make sure that such JIRAs belong to 'test' component. And we can use something like 'test framework (Herriot)' in the description.

          Show
          Konstantin Boudnik added a comment - Well, first we need to make sure that such JIRAs belong to 'test' component. And we can use something like 'test framework (Herriot)' in the description.
          Hide
          Konstantin Boudnik added a comment -

          I have just committed it. Thank you Vinay.

          Show
          Konstantin Boudnik added a comment - I have just committed it. Thank you Vinay.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #278 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/278/)
          HADOOP-6788. [Herriot] Exception exclusion functionality is not working
          correctly. Contributed by Vinay Kumar Thota.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #278 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/278/ ) HADOOP-6788 . [Herriot] Exception exclusion functionality is not working correctly. Contributed by Vinay Kumar Thota.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #356 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/356/)
          HADOOP-6788. [Herriot] Exception exclusion functionality is not working
          correctly. Contributed by Vinay Kumar Thota.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #356 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/356/ ) HADOOP-6788 . [Herriot] Exception exclusion functionality is not working correctly. Contributed by Vinay Kumar Thota.

            People

            • Assignee:
              Vinay Kumar Thota
              Reporter:
              Vinay Kumar Thota
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development