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

Usersmap file in gridmix should not fail on empty lines

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.1
    • Fix Version/s: 0.23.3, 2.0.2-alpha
    • Component/s: contrib/gridmix
    • Labels:
      None

      Description

      An empty line (e.g. at the end of the file) in the usersmap file will cause gridmix to fail. Empty lines should be silently ignored.

      1. MAPREDUCE-4297.patch
        2 kB
        Ravi Prakash
      2. MAPREDUCE-4297.patch
        1 kB
        Ravi Prakash
      3. MAPREDUCE-4297.patch
        1.0 kB
        Ravi Prakash

        Activity

        Hide
        Ravi Prakash added a comment -

        Simple enough patch. I ran TestUserResolve and it passed.

        Not including a test because its a trivial fix

        Show
        Ravi Prakash added a comment - Simple enough patch. I ran TestUserResolve and it passed. Not including a test because its a trivial fix
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530238/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//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/12530238/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2424//console This message is automatically generated.
        Hide
        Robert Joseph Evans added a comment -

        The patch makes it so truly empty lines are ignored, which is good.

        But I have a couple of comments.

        1. rawUgi.getLength() == 0

          is no longer needed because it is the same as

          rawUgi.toString().equals("")
        2. I am just a bit conflicted on white space and I would like your opinion on it. An empty line is ignored but a line with a single space or a tab in it will result in a user named ' ' or '\t' which seems bad to me. Similarly a line with white space in front of or after a user name will result in exactly that a user name with white space in it. Do we want to even try to deal with this or not?
        Show
        Robert Joseph Evans added a comment - The patch makes it so truly empty lines are ignored, which is good. But I have a couple of comments. rawUgi.getLength() == 0 is no longer needed because it is the same as rawUgi.toString().equals("") I am just a bit conflicted on white space and I would like your opinion on it. An empty line is ignored but a line with a single space or a tab in it will result in a user named ' ' or '\t' which seems bad to me. Similarly a line with white space in front of or after a user name will result in exactly that a user name with white space in it. Do we want to even try to deal with this or not?
        Hide
        Ravi Prakash added a comment -

        Thanks for reviewing this Bobby!

        I wasn't sure because I was thinking a byte in Text might decode to whitespace but its getLength() could possibly not be 0. But you are right! We want to ignore those cases. I've removed that condition, and trimmed the strings before comparing and assigning.

        Show
        Ravi Prakash added a comment - Thanks for reviewing this Bobby! I wasn't sure because I was thinking a byte in Text might decode to whitespace but its getLength() could possibly not be 0. But you are right! We want to ignore those cases. I've removed that condition, and trimmed the strings before comparing and assigning.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530275/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//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/12530275/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2425//console This message is automatically generated.
        Hide
        Ravi Gummadi added a comment -

        Code changes look good to me. But the comments/javadoc and the documentation in gridmix.xml mention that each line should be of the form user[,group]*. This needs to be changed as per this patch ?

        Show
        Ravi Gummadi added a comment - Code changes look good to me. But the comments/javadoc and the documentation in gridmix.xml mention that each line should be of the form user [,group] *. This needs to be changed as per this patch ?
        Hide
        Robert Joseph Evans added a comment -

        Canceling patch so we can address the latest comments.

        Show
        Robert Joseph Evans added a comment - Canceling patch so we can address the latest comments.
        Hide
        Ravi Prakash added a comment -

        Thanks for pointing that out RaviG. This patch addresses your comment

        Show
        Ravi Prakash added a comment - Thanks for pointing that out RaviG. This patch addresses your comment
        Hide
        Robert Joseph Evans added a comment -

        Thanks Ravi, +1 I put this into trunk, branch-2, and branch-0.23

        Show
        Robert Joseph Evans added a comment - Thanks Ravi, +1 I put this into trunk, branch-2, and branch-0.23
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530398/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix.

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//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/12530398/MAPREDUCE-4297.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 appears to introduce 11 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-tools/hadoop-gridmix. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-gridmix.html Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2428//console This message is automatically generated.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2377 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2377/)
        MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763)

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

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2377 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2377/ ) MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344763 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2305 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2305/)
        MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763)

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

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2305 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2305/ ) MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344763 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2322 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2322/)
        MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763)

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

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2322 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2322/ ) MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344763 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1063 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1063/)
        MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763)

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

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1063 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1063/ ) MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344763 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #274 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/274/)
        svn merge -c 1344763 FIXES: MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344765)

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

        • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/branches/branch-0.23/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #274 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/274/ ) svn merge -c 1344763 FIXES: MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344765) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344765 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/branches/branch-0.23/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1097 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1097/)
        MAPREDUCE-4297. Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763)

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

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml
        • /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1097 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1097/ ) MAPREDUCE-4297 . Usersmap file in gridmix should not fail on empty lines (Ravi Prakash via bobby) (Revision 1344763) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1344763 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/src/docs/src/documentation/content/xdocs/gridmix.xml /hadoop/common/trunk/hadoop-tools/hadoop-gridmix/src/main/java/org/apache/hadoop/mapred/gridmix/RoundRobinUserResolver.java

          People

          • Assignee:
            Ravi Prakash
            Reporter:
            Ravi Prakash
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development