Uploaded image for project: 'Hive'
  1. Hive
  2. HIVE-2017

Driver.execute() should maintaining SessionState in case of runtime errors

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Not A Problem
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Here's a snippet from Driver.execute():

                  // TODO: This error messaging is not very informative. Fix that.
                  errorMessage = "FAILED: Execution Error, return code " + exitVal + " from "
                      + tsk.getClass().getName();
                  SQLState = "08S01";
                  console.printError(errorMessage);
                  if (running.size() != 0) {
                    taskCleanup();
                  }
                  return 9;
      

      I simply returned in case of runtime errors without maintaining SessionState. It could cause resource leak mentioned in HIVE-1959.

      1. HIVE-2017.1.patch
        9 kB
        Chinna Rao Lalam
      2. HIVE-2017.2.patch
        9 kB
        Chinna Rao Lalam
      3. HIVE-2017.3.patch
        9 kB
        Chinna Rao Lalam
      4. HIVE-2017.4.patch
        10 kB
        Chinna Rao Lalam

        Issue Links

          Activity

          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          taskCleanup() is calling System.exit(9) it will exit from JVM. Here instead of calling
          System.exit(9) we need to kill the map-reduce & non MR processes

          Show
          chinnalalam Chinna Rao Lalam added a comment - taskCleanup() is calling System.exit(9) it will exit from JVM. Here instead of calling System.exit(9) we need to kill the map-reduce & non MR processes
          Hide
          jvs John Sichi added a comment -

          Yeah, see my comments in HIVE-1872 regarding the brokenness of the System.exit

          Show
          jvs John Sichi added a comment - Yeah, see my comments in HIVE-1872 regarding the brokenness of the System.exit
          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.

          Show
          chinnalalam Chinna Rao Lalam added a comment - while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1609/
          -----------------------------------------------------------

          Review request for hive, John Sichi and Ning Zhang.

          Summary
          -------

          while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.

          This addresses bug HIVE-2017.
          https://issues.apache.org/jira/browse/HIVE-2017

          Diffs


          trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1160102
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1160102
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1160102
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1160102
          trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1160102
          trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION
          trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION

          Diff: https://reviews.apache.org/r/1609/diff

          Testing
          -------

          Added test case for this scenario. Ran existing test cases

          Thanks,

          chinna

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1609/ ----------------------------------------------------------- Review request for hive, John Sichi and Ning Zhang. Summary ------- while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job. This addresses bug HIVE-2017 . https://issues.apache.org/jira/browse/HIVE-2017 Diffs trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1160102 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1160102 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1160102 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1160102 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1160102 trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION Diff: https://reviews.apache.org/r/1609/diff Testing ------- Added test case for this scenario. Ran existing test cases Thanks, chinna
          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          Patch become stale i will rebase the patch and i will upload

          Show
          chinnalalam Chinna Rao Lalam added a comment - Patch become stale i will rebase the patch and i will upload
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1609/
          -----------------------------------------------------------

          (Updated 2011-09-16 17:33:41.236186)

          Review request for hive, John Sichi and Ning Zhang.

          Changes
          -------

          Rebased the patch on the latest trunk

          Summary
          -------

          while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.

          This addresses bug HIVE-2017.
          https://issues.apache.org/jira/browse/HIVE-2017

          Diffs (updated)


          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1170977
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1170977
          trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1170977
          trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION
          trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION
          trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1170977
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1170977

          Diff: https://reviews.apache.org/r/1609/diff

          Testing
          -------

          Added test case for this scenario. Ran existing test cases

          Thanks,

          chinna

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1609/ ----------------------------------------------------------- (Updated 2011-09-16 17:33:41.236186) Review request for hive, John Sichi and Ning Zhang. Changes ------- Rebased the patch on the latest trunk Summary ------- while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job. This addresses bug HIVE-2017 . https://issues.apache.org/jira/browse/HIVE-2017 Diffs (updated) trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1170977 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1170977 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1170977 trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1170977 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1170977 Diff: https://reviews.apache.org/r/1609/diff Testing ------- Added test case for this scenario. Ran existing test cases Thanks, chinna
          Hide
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/1609/
          -----------------------------------------------------------

          (Updated 2011-11-03 16:01:16.713162)

          Review request for hive, John Sichi and Ning Zhang.

          Changes
          -------

          Patch becomes stale so rebased

          Summary
          -------

          while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.

          This addresses bug HIVE-2017.
          https://issues.apache.org/jira/browse/HIVE-2017

          Diffs (updated)


          trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1197183
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1197183
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1197183
          trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1197183
          trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1197183
          trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION
          trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION

          Diff: https://reviews.apache.org/r/1609/diff

          Testing
          -------

          Added test case for this scenario. Ran existing test cases

          Thanks,

          chinna

          Show
          jiraposter@reviews.apache.org jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1609/ ----------------------------------------------------------- (Updated 2011-11-03 16:01:16.713162) Review request for hive, John Sichi and Ning Zhang. Changes ------- Patch becomes stale so rebased Summary ------- while processing two parallel tasks if one of the task fails the Driver.taskCleanup() will call system.exit() this will shutdown the jvm so it is replaced with the logic to stop the remaining tasks.. Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job. This addresses bug HIVE-2017 . https://issues.apache.org/jira/browse/HIVE-2017 Diffs (updated) trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1197183 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 1197183 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HadoopJobExecHelper.java 1197183 trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 1197183 trunk/ql/src/java/org/apache/hadoop/hive/ql/io/rcfile/merge/BlockMergeTask.java 1197183 trunk/ql/src/test/queries/clientnegative/alter_exit.q PRE-CREATION trunk/ql/src/test/results/clientnegative/alter_exit.q.out PRE-CREATION Diff: https://reviews.apache.org/r/1609/diff Testing ------- Added test case for this scenario. Ran existing test cases Thanks, chinna
          Hide
          namit Namit Jain added a comment -

          @Chinna, can you refresh it again ?
          This is leading to conflicts - I will look at it

          Show
          namit Namit Jain added a comment - @Chinna, can you refresh it again ? This is leading to conflicts - I will look at it
          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          I will rebase now and i will upload patch.

          Show
          chinnalalam Chinna Rao Lalam added a comment - I will rebase now and i will upload patch.
          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          Hi Namit,

          Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job.

          Here for the non-mr tasks thread.interrupt() may not help completely because they can catch InterruptedException and continue and in some scenarios nothing it will do (As per API documentation) other than just setting interrupt status to true for the thread these cases it will not stop the thread.

          So we can add one variable like "isInterrupted" in Task.java and set it to true when we want to stop this (Ex in taskCleanup()) while execution of task before doing any major operation check this variable isInterrupted and if it is true return immediately but with this approach we need add more checks for isInterrupted statements in all tasks.

          So we need to add statements for checking isInterrupted is set or not in all the tasks before doing any operation it will take some good amount of time(Ex in ExecDriver before submitting the job) pls give u r inputs on this..

          Show
          chinnalalam Chinna Rao Lalam added a comment - Hi Namit, Here need to cleanup 2 kinds of tasks one is non-mr tasks and mr tasks. For stopping non-mr tasks used the thread.interrupt() because every non-mr task will be executed as a thread and for mr tasks maintained a variable called jobKillUri's this variable will track the spawned job kill uri, in taskCleanup() if it is a mr task using this variable kill the job. Here for the non-mr tasks thread.interrupt() may not help completely because they can catch InterruptedException and continue and in some scenarios nothing it will do (As per API documentation) other than just setting interrupt status to true for the thread these cases it will not stop the thread. So we can add one variable like "isInterrupted" in Task.java and set it to true when we want to stop this (Ex in taskCleanup()) while execution of task before doing any major operation check this variable isInterrupted and if it is true return immediately but with this approach we need add more checks for isInterrupted statements in all tasks. So we need to add statements for checking isInterrupted is set or not in all the tasks before doing any operation it will take some good amount of time(Ex in ExecDriver before submitting the job) pls give u r inputs on this..
          Hide
          chinnalalam Chinna Rao Lalam added a comment -

          This is an old issue. This is not an issue on the master.

          Show
          chinnalalam Chinna Rao Lalam added a comment - This is an old issue. This is not an issue on the master.

            People

            • Assignee:
              chinnalalam Chinna Rao Lalam
              Reporter:
              nzhang Ning Zhang
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development