[Hadoop-common-dev] [jira] Created: (HADOOP-5020) JobStatus of killed JobInProgress does carry forward start time of the Job

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663293#action_12663293 ]

Sreekanth Ramakrishnan commented on HADOOP-5020:
------------------------------------------------

Steps to reproduce.

Submit a job.
Wait for the job to Initialize.
Kill the job after initialization.
Check the start time set on JobStatus associated with the Job in progress. The start time is set to zero.



Marking this as major, because removal of killed jobs in Capacity scheduler depends on correctness of the JobStatus.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663296#action_12663296 ]

Amar Kamat commented on HADOOP-5020:
------------------------------------

Looks like a bug in {{JobInProgress.TerminateJob()}} where a new job status is created. Ideally I think we should just do a jobstatus.set() instead of creating a new job-status.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663315#action_12663315 ]

Vivek Ratan commented on HADOOP-5020:
-------------------------------------

bq. Marking this as major, because removal of killed jobs in Capacity scheduler depends on correctness of the JobStatus.

Can you explain? How exactly is the Capacity Scheduler being affected?

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12663723#action_12663723 ]

Sreekanth Ramakrishnan commented on HADOOP-5020:
------------------------------------------------

In JobInitializationPoller, we do a check for the jobs which have been killed/scheduled at start of every polling in-order to clean up the jobs from the job queue. The way the job is removed from the TreeMap is by doing following:
{code}
jobList.remove(new JobSchedulingInfo(jobInProgress));
{code}

The JobSchedulingInfo requires job status which is present in the JobInProgress while construction of the object. When we do a remove from tree map, since the scheduling information differ because of zero stat time, it fails.

The scheduler core does not rely on this way of job removal as it listens to job status changed events in which old and new job status are passed so that takes care of removal from running jobs queue which scheduler maintains.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12664054#action_12664054 ]

Vivek Ratan commented on HADOOP-5020:
-------------------------------------

I've captured how this bug affects the CapacitY Scheduler in Hadoop-5048. This bug is independent of the Capacity Scheduler. Changing the job's start time can affect accounting-related functionality and information.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Amar Kamat updated HADOOP-5020:
-------------------------------

Summary: JobStatus of killed JobInProgress does not carry forward start time of the Job (was: JobStatus of killed JobInProgress does carry forward start time of the Job)

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Amar Kamat updated HADOOP-5020:
-------------------------------

Attachment: HADOOP-5020-v1.0.patch

Attaching a simple straightforward fix that addresses the issue. Instead of creating a new JobStatus object, setting the relevant fields of the old status.
Result of _test-patch_
{noformat}
[exec] +1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] +1 tests included. The patch appears to include 6 new or modified tests.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec]
[exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
{noformat}

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Amar Kamat updated HADOOP-5020:
-------------------------------

Assignee: Amar Kamat
Status: Patch Available (was: Open)

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sreekanth Ramakrishnan updated HADOOP-5020:
-------------------------------------------

Description: If an initialized job is killed, the jobs start time is set to zero. Job status don't seem to have user name set properly. (was: If an initialized job is killed, the jobs start time is set to zero.)
Summary: JobStatus should contain user name and carry forward start time when job is killed. (was: JobStatus of killed JobInProgress does not carry forward start time of the Job)

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673167#action_12673167 ]

Sreekanth Ramakrishnan commented on HADOOP-5020:
------------------------------------------------

I took a quick look at the patch attached. Following are comments which I had

* We are setting username in job status in this patch which was previous not set, I think we should change the JIRA description to reflect change which is being bought by this patch.
* Test case {{testJobStatus()}}, shuts down the cluster bought up by {{TestCase.setup}} method and starts a new cluster, we should avoid this in order to save running time.
* There is a change in JobTracker, which I think is not required for this patch.
* As far as we are testing all fields in {{JobStatus}} why don't we do a sanity check on {{JobStatus.getFinishTime()}} once we finish a job.
* We are waiting for all {{TaskTracker}} to register with {{JobTracker}} in test case, I think this should be moved from the test case to {{MiniMRCluster}} instead of placing it within the test.
* Finally, a suggestion, we should slowly move all usage of {{UtilsForTests}} to {{ControlledMapReduceJob}} and add functionality in it to control job setup and cleanup so that it can be used for testing. I think we should discuss that in seperate JIRA.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12673470#action_12673470 ]

Hadoop QA commented on HADOOP-5020:
-----------------------------------

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12398068/HADOOP-5020-v1.0.patch
against trunk revision 744224.

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

+1 tests included. The patch appears to include 6 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 findbugs. The patch does not introduce any new Findbugs warnings.

+1 Eclipse classpath. The patch retains Eclipse classpath integrity.

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

+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/3851/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3851/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3851/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3851/console

This message is automatically generated.

[ https://issues.apache.org/jira/browse/HADOOP-5020?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Douglas updated HADOOP-5020:
----------------------------------

Status: Open (was: Patch Available)

Cancelling patch while Sreekanth's comments are addressed