Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0, 0.24.0
    • Fix Version/s: 0.23.2
    • Component/s: fs
    • Labels:
      None

      Description

      Control-c is apparently generating InterruptedIOExceptions. The path processing loop traps IOException, displays the error, and moves to the next path. This means that recursive operations cannot be aborted.

        Activity

        Hide
        Daryn Sharp added a comment -

        convert InterruptedIOException into a runtime exception and catch it in the run method

        Show
        Daryn Sharp added a comment - convert InterruptedIOException into a runtime exception and catch it in the run method
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12517334/HADOOP-8146.patch
        against trunk revision .

        +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 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 .

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/681//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/681//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/12517334/HADOOP-8146.patch against trunk revision . +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 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 . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/681//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/681//console This message is automatically generated.
        Hide
        Uma Maheswara Rao G added a comment -

        Hi Daryn,
        It make sense to handle interrupt exceptions as you mentioned it is creating problem with recursive operations.

        I reviewed your patch, few comments.

        1) Instead of hardcoding return codes, why can't we create some enum with exitcodes for more readability?

        something like this

        enum ExitCodes {
            SUCCESS(0), ERROR(1), INTERRUPT(130);
            private int exitCode;
        
            private ExitCodes(int exitCode) {
              this.exitCode = exitCode;
            }
        
            public int getValue() {
              return this.exitCode;
            }
          }
        

        2) Could you please add a test with recursive operations by extending some existing command class and put your counter for testing and register that command. what do you say?

        Show
        Uma Maheswara Rao G added a comment - Hi Daryn, It make sense to handle interrupt exceptions as you mentioned it is creating problem with recursive operations. I reviewed your patch, few comments. 1) Instead of hardcoding return codes, why can't we create some enum with exitcodes for more readability? something like this enum ExitCodes { SUCCESS(0), ERROR(1), INTERRUPT(130); private int exitCode; private ExitCodes( int exitCode) { this .exitCode = exitCode; } public int getValue() { return this .exitCode; } } 2) Could you please add a test with recursive operations by extending some existing command class and put your counter for testing and register that command. what do you say?
        Hide
        Daryn Sharp added a comment -
        1. I think it's overall a good idea, but dfsadmin is still subclassing FsShell/Command. It's not using the rest of the logic, but I'd rather not change nor risk destabilizing that command at this time. Would you be willing to file a new jira?
        2. I wish I could, but the commands do not have public visibility to prevent direct instantiation. All the loops for processing paths including a try loop that catches IOException, so interrupts are always converted to exceptions. The 2nd interrupt test does already exercise recursion into a directory.

        Let me know if you want me to make further changes.

        Show
        Daryn Sharp added a comment - I think it's overall a good idea, but dfsadmin is still subclassing FsShell/Command. It's not using the rest of the logic, but I'd rather not change nor risk destabilizing that command at this time. Would you be willing to file a new jira? I wish I could, but the commands do not have public visibility to prevent direct instantiation. All the loops for processing paths including a try loop that catches IOException, so interrupts are always converted to exceptions. The 2nd interrupt test does already exercise recursion into a directory. Let me know if you want me to make further changes.
        Hide
        Uma Maheswara Rao G added a comment -

        I think it's overall a good idea, but dfsadmin is still subclassing FsShell/Command. It's not using the rest of the logic, but I'd rather not change nor risk destabilizing that command at this time. Would you be willing to file a new jira?

        Yes, please file it.

        Agree with your second point.

        +1 for the patch.

        Show
        Uma Maheswara Rao G added a comment - I think it's overall a good idea, but dfsadmin is still subclassing FsShell/Command. It's not using the rest of the logic, but I'd rather not change nor risk destabilizing that command at this time. Would you be willing to file a new jira? Yes, please file it. Agree with your second point. +1 for the patch.
        Hide
        Uma Maheswara Rao G added a comment -

        I will commit this patch tomorrow if there are no other comments.

        Show
        Uma Maheswara Rao G added a comment - I will commit this patch tomorrow if there are no other comments.
        Hide
        Daryn Sharp added a comment -

        Filed HADOOP-8155 for the enum improvement. Thanks!

        Show
        Daryn Sharp added a comment - Filed HADOOP-8155 for the enum improvement. Thanks!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #1933 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1933/)
        HADOOP-8146. FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976)

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

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1933 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1933/ ) HADOOP-8146 . FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976) Result = SUCCESS umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #1858 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1858/)
        HADOOP-8146. FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976)

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

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1858 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1858/ ) HADOOP-8146 . FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976) Result = SUCCESS umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Hide
        Uma Maheswara Rao G added a comment -

        Just Committed to Trunk. Thanks a lot to Daryn!
        Will merge it to 23.2 as well.

        Show
        Uma Maheswara Rao G added a comment - Just Committed to Trunk. Thanks a lot to Daryn! Will merge it to 23.2 as well.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #1867 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1867/)
        HADOOP-8146. FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976)

        Result = ABORTED
        umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1867 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1867/ ) HADOOP-8146 . FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976) Result = ABORTED umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Hide
        Uma Maheswara Rao G added a comment -

        Merged to 23.2 also. Thanks!

        Show
        Uma Maheswara Rao G added a comment - Merged to 23.2 also. Thanks!
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #980 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/980/)
        HADOOP-8146. FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976)

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

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #980 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/980/ ) HADOOP-8146 . FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976) Result = SUCCESS umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1015 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1015/)
        HADOOP-8146. FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976)

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

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1015 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1015/ ) HADOOP-8146 . FsShell commands cannot be interrupted. Contributed by Daryn Shar (Revision 1298976) Result = SUCCESS umamahesh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1298976 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Command.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShellReturnCode.java

          People

          • Assignee:
            Daryn Sharp
            Reporter:
            Daryn Sharp
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development