Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2-alpha, 0.23.5
    • Fix Version/s: 2.0.3-alpha, 0.23.6
    • Component/s: examples
    • Labels:
      None

      Description

      MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino, although it seems to carry a bug when fetching values.

      This JIRA is for fixing that on trunk and backporting it onto branches.

      1. mr-4925-trunk.patch
        2 kB
        Karthik Kambatla
      2. mr-4925-branch2.patch
        2 kB
        Karthik Kambatla
      3. mr-4925-branch-0.23.patch
        2 kB
        Karthik Kambatla
      4. mr-4925.patch
        2 kB
        Karthik Kambatla
      5. mr-4925.patch
        2 kB
        Karthik Kambatla

        Issue Links

          Activity

          Karthik Kambatla (Inactive) created issue -
          Karthik Kambatla (Inactive) made changes -
          Field Original Value New Value
          Affects Version/s 1.1.1 [ 12321660 ]
          Karthik Kambatla (Inactive) made changes -
          Link This issue depends on MAPREDUCE-4678 [ MAPREDUCE-4678 ]
          Karthik Kambatla (Inactive) made changes -
          Description MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino. This JIRA is for branch-1.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Uploading a trivial patch.

          Show
          Karthik Kambatla (Inactive) added a comment - Uploading a trivial patch.
          Karthik Kambatla (Inactive) made changes -
          Attachment mr-4925.patch [ 12563722 ]
          Karthik Kambatla (Inactive) made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563722/mr-4925.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3211//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/12563722/mr-4925.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3211//console This message is automatically generated.
          Hide
          Surenkumar Nihalani added a comment -

          I was reviewing the patch and

          m4-4295.patch.java
          +    for (int i = 0; i < args.length; i++) {
          +      if (args[i].equalsIgnoreCase("-depth")) {
          +        depth = Integer.parseInt(args[i++].trim());
          +      } else if (args[i].equalsIgnoreCase("-height")) {
          +        height = Integer.parseInt(args[i++].trim());
          +      } else if (args[i].equalsIgnoreCase("-width")) {
          +        width = Integer.parseInt(args[i++].trim());
          +      }
          +    }
          

          I don't think this works. I believe we need to access the parameters like args[++i] instead of args[i++]
          Can you confirm this worked?

          Show
          Surenkumar Nihalani added a comment - I was reviewing the patch and m4-4295.patch.java + for ( int i = 0; i < args.length; i++) { + if (args[i].equalsIgnoreCase( "-depth" )) { + depth = Integer .parseInt(args[i++].trim()); + } else if (args[i].equalsIgnoreCase( "-height" )) { + height = Integer .parseInt(args[i++].trim()); + } else if (args[i].equalsIgnoreCase( "-width" )) { + width = Integer .parseInt(args[i++].trim()); + } + } I don't think this works. I believe we need to access the parameters like args [++i] instead of args [i++] Can you confirm this worked?
          Hide
          Robert Joseph Evans added a comment -

          Before this goes into 1.0 it really needs to be pulled into 2.0 and 0.23 too, otherwise there will be some regressions.

          Show
          Robert Joseph Evans added a comment - Before this goes into 1.0 it really needs to be pulled into 2.0 and 0.23 too, otherwise there will be some regressions.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Thanks Bobby. That makes sense. Let me upload patches for branch-2 and branch-0.23 also here.

          Show
          Karthik Kambatla (Inactive) added a comment - Thanks Bobby. That makes sense. Let me upload patches for branch-2 and branch-0.23 also here.
          Karthik Kambatla (Inactive) made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Karthik Kambatla (Inactive) added a comment -

          MAPREDUCE-4678 still suffers from a couple of issues:

          1. It introduces a regression in the sense that the user can't pass the arguments via -Dmapreduce.pentomino.depth etc.
          2. As Suren pointed out, passing the command line args through -depth, -width, -height is also broken.

          Uploading mr-4925-trunk.patch to address these.

          Once Jenkins +1s it, I ll upload the patches for branch-2, branch-0.23 and branch-1.

          Show
          Karthik Kambatla (Inactive) added a comment - MAPREDUCE-4678 still suffers from a couple of issues: It introduces a regression in the sense that the user can't pass the arguments via -Dmapreduce.pentomino.depth etc. As Suren pointed out, passing the command line args through -depth, -width, -height is also broken. Uploading mr-4925-trunk.patch to address these. Once Jenkins +1s it, I ll upload the patches for branch-2, branch-0.23 and branch-1.
          Karthik Kambatla (Inactive) made changes -
          Attachment mr-4925-trunk.patch [ 12563824 ]
          Karthik Kambatla (Inactive) made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Affects Version/s 0.23.5 [ 12323312 ]
          Affects Version/s 2.0.2-alpha [ 12322471 ]
          Hide
          Karthik Kambatla (Inactive) added a comment -

          For the trunk patch, I ran the example with arguments passed via both -depth format, and -Dmapreduce.pentomino.depth format, and no arguments (only <output>).

          Show
          Karthik Kambatla (Inactive) added a comment - For the trunk patch, I ran the example with arguments passed via both -depth format, and -Dmapreduce.pentomino.depth format, and no arguments (only <output>).
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563824/mr-4925-trunk.patch
          against trunk revision .

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-examples.

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

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3212//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3212//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/12563824/mr-4925-trunk.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-examples. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3212//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3212//console This message is automatically generated.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Uploading patches for branch-2, branch-0.23 and branch-1

          Show
          Karthik Kambatla (Inactive) added a comment - Uploading patches for branch-2, branch-0.23 and branch-1
          Karthik Kambatla (Inactive) made changes -
          Attachment mr-4925.patch [ 12563890 ]
          Attachment mr-4925-branch2.patch [ 12563891 ]
          Attachment mr-4925-branch-0.23.patch [ 12563892 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12563892/mr-4925-branch-0.23.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3216//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/12563892/mr-4925-branch-0.23.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3216//console This message is automatically generated.
          Harsh J made changes -
          Summary Backport MAPREDUCE-4678 to branch-1 The pentomino option parser may be buggy
          Harsh J made changes -
          Description MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino. This JIRA is for branch-1. MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino, although it seems to carry a bug when fetching values.

          This JIRA is for fixing that on trunk and backporting the full fix onto other branches.
          Hide
          Harsh J added a comment -

          Thanks Karthik,

          The trunk patch looks good, and the complete backports. I'm updating my local copies, and will commit them shortly. +1

          Show
          Harsh J added a comment - Thanks Karthik, The trunk patch looks good, and the complete backports. I'm updating my local copies, and will commit them shortly. +1
          Hide
          Harsh J added a comment -

          Karthik,

          Committing backports of two fixes at once (this and the MAPREDUCE-4678 one) may not be a clean thing to do combined. Do you mind spinning off a new JIRA for the MAPREDUCE-4678 backport and leave this be for patches for the fix atop that jira (the opt parse bug)?

          Let me know if you're strapped on time and I'll handle it for you.

          Show
          Harsh J added a comment - Karthik, Committing backports of two fixes at once (this and the MAPREDUCE-4678 one) may not be a clean thing to do combined. Do you mind spinning off a new JIRA for the MAPREDUCE-4678 backport and leave this be for patches for the fix atop that jira (the opt parse bug)? Let me know if you're strapped on time and I'll handle it for you.
          Karthik Kambatla (Inactive) made changes -
          Link This issue is blocked by MAPREDUCE-4930 [ MAPREDUCE-4930 ]
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Hi Harsh

          Thanks for taking a look. I created MAPREDUCE-4930 for this to follow your suggestion, but two patches for branch-1 seems like an overkill. Here is what I propose:

          1. Re-open MAPREDUCE-4678, and commit the patch there to branch-2 and branch-0.23
          2. This JIRA (MAPREDUCE-4925) fixes the bug introduced by 4678.
          3. In MAPREDUCE-4930, we can backport the above two in a single patch.

          This way, we can make sure the commits are same on trunk, branch-2 and branch-0.23, so that all merges and stuff work well. We need not introduce a bug unnecessarily in branch-1, the example code and the entire code base is different, and it is in maintenance mode.

          Hopefully, this is reasonable. Let me know if you think otherwise.

          Show
          Karthik Kambatla (Inactive) added a comment - Hi Harsh Thanks for taking a look. I created MAPREDUCE-4930 for this to follow your suggestion, but two patches for branch-1 seems like an overkill. Here is what I propose: Re-open MAPREDUCE-4678 , and commit the patch there to branch-2 and branch-0.23 This JIRA ( MAPREDUCE-4925 ) fixes the bug introduced by 4678. In MAPREDUCE-4930 , we can backport the above two in a single patch. This way, we can make sure the commits are same on trunk, branch-2 and branch-0.23, so that all merges and stuff work well. We need not introduce a bug unnecessarily in branch-1, the example code and the entire code base is different, and it is in maintenance mode. Hopefully, this is reasonable. Let me know if you think otherwise.
          Hide
          Karthik Kambatla (Inactive) added a comment -

          Once MR-4678 is applied to branch-2 and branch-0.23, the trunk patch here applies to those branches as well. Once it is committed, I ll delete the incorrect patches for branch-2 and branch-0.23.

          Show
          Karthik Kambatla (Inactive) added a comment - Once MR-4678 is applied to branch-2 and branch-0.23, the trunk patch here applies to those branches as well. Once it is committed, I ll delete the incorrect patches for branch-2 and branch-0.23.
          Karthik Kambatla (Inactive) made changes -
          Link This issue is blocked by MAPREDUCE-4930 [ MAPREDUCE-4930 ]
          Karthik Kambatla (Inactive) made changes -
          Link This issue blocks MAPREDUCE-4930 [ MAPREDUCE-4930 ]
          Karthik Kambatla (Inactive) made changes -
          Affects Version/s 1.1.1 [ 12321660 ]
          Harsh J made changes -
          Description MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino, although it seems to carry a bug when fetching values.

          This JIRA is for fixing that on trunk and backporting the full fix onto other branches.
          MAPREDUCE-4678 adds an easier way to specify arguments to Pentomino, although it seems to carry a bug when fetching values.

          This JIRA is for fixing that on trunk and backporting it onto branches.
          Hide
          Harsh J added a comment -
          Show
          Harsh J added a comment - +1 on trunk diff patch https://issues.apache.org/jira/secure/attachment/12563824/mr-4925-trunk.patch . Committing.
          Hide
          Harsh J added a comment -

          Committed to trunk, branch-2 and branch-0.23. Thanks!

          Show
          Harsh J added a comment - Committed to trunk, branch-2 and branch-0.23. Thanks!
          Harsh J made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 2.0.3-alpha [ 12323275 ]
          Fix Version/s 0.23.6 [ 12323502 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3235 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3235/)
          MAPREDUCE-4925. The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3235 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3235/ ) MAPREDUCE-4925 . The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #98 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/98/)
          MAPREDUCE-4925. The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414)

          Result = SUCCESS
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #98 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/98/ ) MAPREDUCE-4925 . The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414) Result = SUCCESS harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-0.23-Build #496 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/496/)
          MAPREDUCE-4925. The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433417)

          Result = FAILURE
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433417
          Files :

          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #496 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/496/ ) MAPREDUCE-4925 . The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433417) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433417 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1287 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1287/)
          MAPREDUCE-4925. The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414)

          Result = FAILURE
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1287 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1287/ ) MAPREDUCE-4925 . The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1315 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1315/)
          MAPREDUCE-4925. The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414)

          Result = FAILURE
          harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1315 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1315/ ) MAPREDUCE-4925 . The pentomino option parser may be buggy. Contributed by Karthik Kambatla. (harsh) (Revision 1433414) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1433414 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-examples/src/main/java/org/apache/hadoop/examples/dancing/DistributedPentomino.java
          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Link This issue depends on MAPREDUCE-4678 [ MAPREDUCE-4678 ]
          Gavin made changes -
          Link This issue depends upon MAPREDUCE-4678 [ MAPREDUCE-4678 ]
          Gavin made changes -
          Assignee Karthik Kambatla [ kkambatl ] Karthik Kambatla [ kasha ]
          Gavin made changes -
          Reporter Karthik Kambatla [ kkambatl ] Karthik Kambatla [ kasha ]

            People

            • Assignee:
              Karthik Kambatla
              Reporter:
              Karthik Kambatla
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development