Details

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

      Description

      Counters are very useful because of the logging and transmitting are already there.
      It is very convenient to transmit and store numbers. But currently Counter only has an increment() method.
      It will be nice if there can be a setValue() method in this class that will allow us to transmit wider variety of information through it.

      What do you think?

      1. MAPREDUCE-1762.txt
        2 kB
        Scott Chen
      2. MAPREDUCE-1762.1.txt
        2 kB
        Scott Chen
      3. M1762-2.patch
        3 kB
        Chris Douglas
      4. M1762-javadoc.patch
        0.4 kB
        Scott Chen

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #523 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/523/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #523 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/523/ )
          Hide
          Chris Douglas added a comment -

          I committed the fix to the javadoc.

          Show
          Chris Douglas added a comment - I committed the fix to the javadoc.
          Hide
          Chris Douglas added a comment -

          Sorry, I hadn't placed a watch on this issue and missed the javadoc warning (as did Hudson?)

          I agree, it's not worth reverting this for a minor tweak.

          Show
          Chris Douglas added a comment - Sorry, I hadn't placed a watch on this issue and missed the javadoc warning (as did Hudson?) I agree, it's not worth reverting this for a minor tweak.
          Hide
          Amareshwari Sriramadasu added a comment -

          Looks like the parameter name 'value' is missing from the Javadoc (i.e @param). This is resulting in test-patch failure on trunk.

          javadoc target in test-patch is broken and tracked through MAPREDUCE-1947. Hudson's test-patch builds are not failing (even the patch build for this jira) because of MAPREDUCE-1947.

          Show
          Amareshwari Sriramadasu added a comment - Looks like the parameter name 'value' is missing from the Javadoc (i.e @param). This is resulting in test-patch failure on trunk. javadoc target in test-patch is broken and tracked through MAPREDUCE-1947 . Hudson's test-patch builds are not failing (even the patch build for this jira) because of MAPREDUCE-1947 .
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Please open a new ticket. Reverting the original (big) patch isn't really needed for this trivial fix.

          Show
          Vinod Kumar Vavilapalli added a comment - Please open a new ticket. Reverting the original (big) patch isn't really needed for this trivial fix.
          Hide
          Amar Kamat added a comment -

          Scott,
          Thanks for the quick response. I think its better that we revert the earlier patch and commit the new one. Iterative patches are difficult to track. Could you please run the new (final) patch with Hudson?

          @Chris: What do you think?

          Show
          Amar Kamat added a comment - Scott, Thanks for the quick response. I think its better that we revert the earlier patch and commit the new one. Iterative patches are difficult to track. Could you please run the new (final) patch with Hudson? @Chris: What do you think?
          Hide
          Scott Chen added a comment -

          Fixed the bad javadoc

          Show
          Scott Chen added a comment - Fixed the bad javadoc
          Hide
          Scott Chen added a comment -

          Sorry. That's my bad. Should I make a patch and post it here?

          Show
          Scott Chen added a comment - Sorry. That's my bad. Should I make a patch and post it here?
          Hide
          Amar Kamat added a comment -

          Scott/Chris,
          I see the following piece of code in this patch

          /**
             * Set this counter by the given value
             * @param the value to set
             */
            public synchronized void setValue(long value) {
              this.value = value;
            }
          

          Looks like the parameter name 'value' is missing from the Javadoc (i.e @param). This is resulting in test-patch failure on trunk. Can one of you look into this?

          Show
          Amar Kamat added a comment - Scott/Chris, I see the following piece of code in this patch /** * Set this counter by the given value * @param the value to set */ public synchronized void setValue( long value) { this .value = value; } Looks like the parameter name 'value' is missing from the Javadoc (i.e @param). This is resulting in test-patch failure on trunk. Can one of you look into this?
          Hide
          Scott Chen added a comment -

          Thanks for the help, Chris. I appreciate it

          Show
          Scott Chen added a comment - Thanks for the help, Chris. I appreciate it
          Hide
          Chris Douglas added a comment -

          +1

          I committed this. Thanks, Scott!

          Show
          Chris Douglas added a comment - +1 I committed this. Thanks, Scott!
          Hide
          Chris Douglas added a comment -

          Implemented these minor tweaks; just moved the code and set the assertions.

          Show
          Chris Douglas added a comment - Implemented these minor tweaks; just moved the code and set the assertions.
          Hide
          Chris Douglas added a comment -

          The expected/actual values are backwards in the asserts and the test belongs in o.a.h.mapreduce, but other than that this looks good.

          Show
          Chris Douglas added a comment - The expected/actual values are backwards in the asserts and the test belongs in o.a.h.mapreduce, but other than that this looks good.
          Hide
          Scott Chen added a comment -

          Fixed a typo in the patch.

          Show
          Scott Chen added a comment - Fixed a typo in the patch.
          Hide
          Dmytro Molkov added a comment -

          The code looks good

          Show
          Dmytro Molkov added a comment - The code looks good
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12444361/MAPREDUCE-1762.txt
          against trunk revision 943372.

          +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/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/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/12444361/MAPREDUCE-1762.txt against trunk revision 943372. +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/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h4.grid.sp2.yahoo.net/184/console This message is automatically generated.
          Hide
          Scott Chen added a comment -

          Add a setValue(long) method in Counter.java and a simple unit test.

          Show
          Scott Chen added a comment - Add a setValue(long) method in Counter.java and a simple unit test.

            People

            • Assignee:
              Scott Chen
              Reporter:
              Scott Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development