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

          Scott Chen created issue -
          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.
          Scott Chen made changes -
          Field Original Value New Value
          Attachment MAPREDUCE-1762.txt [ 12444361 ]
          Scott Chen made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          dhruba borthakur made changes -
          Link This issue blocks MAPREDUCE-220 [ MAPREDUCE-220 ]
          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
          Dmytro Molkov added a comment -

          The code looks good

          Show
          Dmytro Molkov added a comment - The code 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.
          Scott Chen made changes -
          Attachment MAPREDUCE-1762.1.txt [ 12444975 ]
          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
          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.
          Chris Douglas made changes -
          Attachment M1762-2.patch [ 12446429 ]
          Hide
          Chris Douglas added a comment -

          +1

          I committed this. Thanks, Scott!

          Show
          Chris Douglas added a comment - +1 I committed this. Thanks, Scott!
          Chris Douglas made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Resolution Fixed [ 1 ]
          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
          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 -

          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
          Scott Chen added a comment -

          Fixed the bad javadoc

          Show
          Scott Chen added a comment - Fixed the bad javadoc
          Scott Chen made changes -
          Attachment M1762-javadoc.patch [ 12451249 ]
          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
          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.
          Amareshwari Sriramadasu made changes -
          Link This issue relates to MAPREDUCE-1947 [ MAPREDUCE-1947 ]
          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
          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
          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
          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/ )
          Konstantin Shvachko made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development