Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-1023

Newly introduced findBugs warnings should be suppressed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: build
    • Labels:
      None

      Description

      FindBugs warnings introduced by MAPREDUCE-711 and HADOOP-6230 should be suppressed by modifying src/test/findbugsExcludeFile.xml.

        Activity

        Hide
        Vinod Kumar Vavilapalli added a comment -

        Created MAPREDUCE-1108 so we don't need to create another issue like this one.

        Show
        Vinod Kumar Vavilapalli added a comment - Created MAPREDUCE-1108 so we don't need to create another issue like this one.
        Hide
        Amareshwari Sriramadasu added a comment -

        Duplicate of MAPREDUCE-769

        Show
        Amareshwari Sriramadasu added a comment - Duplicate of MAPREDUCE-769
        Hide
        Konstantin Boudnik added a comment -

        I don't think that ignoring warnings is a way I'd go. Let's keep them because at some point they have to be fixed (HDFS does such clean-ups and it works great!).

        If the bar is raised than test-patch will start reporting freshly introduced warnings if so happens. When the deprecated warnings are fixed in MR and the total number of warning is brought down to an acceptable level (i.e. below 100?) we'd be able to lower the bar back to the current level.

        I believe 1000 warnings level has been set as something clearly impossible to reach. Well, life proves it wrong

        Show
        Konstantin Boudnik added a comment - I don't think that ignoring warnings is a way I'd go. Let's keep them because at some point they have to be fixed (HDFS does such clean-ups and it works great!). If the bar is raised than test-patch will start reporting freshly introduced warnings if so happens. When the deprecated warnings are fixed in MR and the total number of warning is brought down to an acceptable level (i.e. below 100?) we'd be able to lower the bar back to the current level. I believe 1000 warnings level has been set as something clearly impossible to reach. Well, life proves it wrong
        Hide
        Jothi Padmanabhan added a comment -

        IF we are making a MR specific change, then we might as well ignore all deprecated warnings using Xlint:-deprecation. This will print all warnings that are not deprecation related and we could do the normal compare, no?

        Show
        Jothi Padmanabhan added a comment - IF we are making a MR specific change, then we might as well ignore all deprecated warnings using Xlint:-deprecation. This will print all warnings that are not deprecation related and we could do the normal compare, no?
        Hide
        Konstantin Boudnik added a comment -

        Shall we rise the level of the warnings (m.b. for MR only, because it has so many) up to 10,000? Otherwise, the new issues will be effectively swept under a rag time and again...

        Show
        Konstantin Boudnik added a comment - Shall we rise the level of the warnings (m.b. for MR only, because it has so many) up to 10,000? Otherwise, the new issues will be effectively swept under a rag time and again...
        Hide
        Jothi Padmanabhan added a comment -

        There are 11 findbugs warnings (I think introduced by 3 patches, MAPREDUCE-711, MAPREDUCE-885 and HADOOP-5661) in the trunk and about 20+ javac warnings (warnings other than deprecated ones) in the trunk.
        Managing javac warnings is even more difficult as there are thousands of deprecated warnings and any newly added non-deprecated warnings just does not get reported in testpatch as we set maxwarns=1000.

        Show
        Jothi Padmanabhan added a comment - There are 11 findbugs warnings (I think introduced by 3 patches, MAPREDUCE-711 , MAPREDUCE-885 and HADOOP-5661 ) in the trunk and about 20+ javac warnings (warnings other than deprecated ones) in the trunk. Managing javac warnings is even more difficult as there are thousands of deprecated warnings and any newly added non-deprecated warnings just does not get reported in testpatch as we set maxwarns=1000.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        I couldn't locate the project history because of the project split. Well, at any rate, I guess that the problem at that time is people didn't realize what to do when an un-ignorable warning is spelt out.

        I think now, we should bring the number back to zero, advertise the fact that src/test/findbugsExcludeFile.xml serves this purpose, perhaps via the Hudson's reports, and be careful and persistent in making patches follow this. Opinions?

        Show
        Vinod Kumar Vavilapalli added a comment - I couldn't locate the project history because of the project split. Well, at any rate, I guess that the problem at that time is people didn't realize what to do when an un-ignorable warning is spelt out. I think now, we should bring the number back to zero, advertise the fact that src/test/findbugsExcludeFile.xml serves this purpose, perhaps via the Hudson's reports, and be careful and persistent in making patches follow this. Opinions?
        Hide
        Jothi Padmanabhan added a comment -

        How about modifying the Hudson test-patch.sh script to scream when the warnings go above zero

        Well, after findbugs hit 0 with trunk, test-patch.sh would have screamed even when these new findbugs got introduced as the difference between patch and trunk would have been greater than zero. But then, people seem to have ignored the sreams!!

        Show
        Jothi Padmanabhan added a comment - How about modifying the Hudson test-patch.sh script to scream when the warnings go above zero Well, after findbugs hit 0 with trunk, test-patch.sh would have screamed even when these new findbugs got introduced as the difference between patch and trunk would have been greater than zero. But then, people seem to have ignored the sreams!!
        Hide
        Vinod Kumar Vavilapalli added a comment -

        In fact, why don't we just suppress everything that newly appeared after HADOOP-5661 went in?

        Via HADOOP-5661, Jothi went through pains to make sure findBugs warnings become zero. All that effort would be a waste if patches keep ignoring these warnings. How about modifying the Hudson test-patch.sh script to scream when the warnings go above zero level and making sure trunk is always at zero findBugs warnings? As noted by others, this would also speed up the test-patch.sh process as we will just to need to find the number of warnings introduced by the patch.

        I am not very sure whether this should be fixed as bug/blocker for 0.21. Thoughts?

        Show
        Vinod Kumar Vavilapalli added a comment - In fact, why don't we just suppress everything that newly appeared after HADOOP-5661 went in? Via HADOOP-5661 , Jothi went through pains to make sure findBugs warnings become zero. All that effort would be a waste if patches keep ignoring these warnings. How about modifying the Hudson test-patch.sh script to scream when the warnings go above zero level and making sure trunk is always at zero findBugs warnings? As noted by others, this would also speed up the test-patch.sh process as we will just to need to find the number of warnings introduced by the patch. I am not very sure whether this should be fixed as bug/blocker for 0.21. Thoughts?

          People

          • Assignee:
            Unassigned
            Reporter:
            Vinod Kumar Vavilapalli
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development