Hadoop Common
  1. Hadoop Common
  2. HADOOP-4420

JobTracker.killJob() doesn't check for the JobID being valid

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.18.0, 0.18.1, 0.18.2
    • Fix Version/s: 0.19.1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      As with HADOOP-4419, JobTracker.killJob() assumes that the supplied JobID always matches to a valid job.

      1. hadoop-4420.patch
        2 kB
        Aaron Kimball
      2. hadoop-4420.patch.2
        3 kB
        Aaron Kimball

        Issue Links

          Activity

          Hide
          Aaron Kimball added a comment -

          This also fixes HADOOP-4419 at the same time

          Show
          Aaron Kimball added a comment - This also fixes HADOOP-4419 at the same time
          Hide
          Aaron Kimball added a comment -

          Should also add: no unit tests, because it's just straightforward null checks.

          Show
          Aaron Kimball added a comment - Should also add: no unit tests, because it's just straightforward null checks.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          For Hadoop-0.20, we should think about ways of informing clients about the failures, instead of just returning silently, as Hemanth pointed out here. This needs API changes and so the approach in the current patch is fine for 0.19.

          Some comments about this patch.

          • You don't have a null check for JobID in setJobPriority.
          • The update you made for Changes.txt is not getting applied cleanly. I think you can leave that to be done by the committer of this patch.
          Show
          Vinod Kumar Vavilapalli added a comment - For Hadoop-0.20, we should think about ways of informing clients about the failures, instead of just returning silently, as Hemanth pointed out here . This needs API changes and so the approach in the current patch is fine for 0.19. Some comments about this patch. You don't have a null check for JobID in setJobPriority. The update you made for Changes.txt is not getting applied cleanly. I think you can leave that to be done by the committer of this patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12393039/hadoop-4420.patch
          against trunk revision 709040.

          +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 tests are needed for this patch.

          +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 Eclipse classpath. The patch retains Eclipse classpath integrity.

          +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/Hadoop-Patch/3511/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/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/12393039/hadoop-4420.patch against trunk revision 709040. +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 tests are needed for this patch. +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 Eclipse classpath. The patch retains Eclipse classpath integrity. +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/Hadoop-Patch/3511/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3511/console This message is automatically generated.
          Hide
          Aaron Kimball added a comment -

          Added null check and log warning on jobid to getJobStatus(); also added null check in o.a.h.m.CompletedJobStore.readJobStatus() which it calls.

          Show
          Aaron Kimball added a comment - Added null check and log warning on jobid to getJobStatus(); also added null check in o.a.h.m.CompletedJobStore.readJobStatus() which it calls.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12393334/hadoop-4420.patch.2
          against trunk revision 720930.

          +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 tests are needed for this patch.

          +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 Eclipse classpath. The patch retains Eclipse classpath integrity.

          -1 core tests. The patch failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/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/12393334/hadoop-4420.patch.2 against trunk revision 720930. +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 tests are needed for this patch. +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 Eclipse classpath. The patch retains Eclipse classpath integrity. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3656/console This message is automatically generated.
          Hide
          Aaron Kimball added a comment -

          core test failure is due to something in DFS testFileAppend – Can't see how it's related to this.

          Show
          Aaron Kimball added a comment - core test failure is due to something in DFS testFileAppend – Can't see how it's related to this.
          Hide
          Tom White added a comment -

          I've just committed this. Thanks Aaron!

          See HADOOP-4725 for a follow-on feature to notify clients when they pass bad job IDs.

          Show
          Tom White added a comment - I've just committed this. Thanks Aaron! See HADOOP-4725 for a follow-on feature to notify clients when they pass bad job IDs.
          Hide
          Hudson added a comment -
          Show
          Hudson added a comment - Integrated in Hadoop-trunk #680 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/680/ )

            People

            • Assignee:
              Aaron Kimball
              Reporter:
              Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development