You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Steve Loughran (JIRA)" <ji...@apache.org> on 2008/10/15 15:13:44 UTC

[jira] Created: (HADOOP-4419) JobTracker.setJobPriority() doesn't check for a jobID lookup failing

JobTracker.setJobPriority() doesn't check for a jobID lookup failing
--------------------------------------------------------------------

                 Key: HADOOP-4419
                 URL: https://issues.apache.org/jira/browse/HADOOP-4419
             Project: Hadoop Core
          Issue Type: Bug
          Components: mapred
    Affects Versions: 0.19.0, 0.20.0
            Reporter: Steve Loughran
            Priority: Minor


Looking at the entry points of the JobTracker API, it seems that JobTracker.setJobPriority()  doesnt expect the JobID lookup ever to return null

It goes straight from lookup to checking access, an operation that assumes that job!=null
    JobInProgress job = jobs.get(jobid);
    checkAccess(job, QueueManager.QueueOperation.ADMINISTER_JOBS);

Recommend: add a test that calls this operaton with an invalid jobID, then fix the code as appropriate

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-4419) JobTracker.setJobPriority() doesn't check for a jobID lookup failing

Posted by "Hemanth Yamijala (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12639887#action_12639887 ] 

Hemanth Yamijala commented on HADOOP-4419:
------------------------------------------

I verified that when the setJobPriority or killJob (see HADOOP-4420) are called from the hadoop cli, a check is made to see if the job exists, and then the operation is launched. If the job doesn't exist, the operation fails with a message like "Could not find job ...". 

That said, there is a window of a chance when after the check, and before the request is submitted, the job is removed. Hence +1 for checking the validity of the job in the two methods.

> JobTracker.setJobPriority() doesn't check for a jobID lookup failing
> --------------------------------------------------------------------
>
>                 Key: HADOOP-4419
>                 URL: https://issues.apache.org/jira/browse/HADOOP-4419
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: mapred
>    Affects Versions: 0.19.0, 0.20.0
>            Reporter: Steve Loughran
>            Priority: Minor
>
> Looking at the entry points of the JobTracker API, it seems that JobTracker.setJobPriority()  doesnt expect the JobID lookup ever to return null
> It goes straight from lookup to checking access, an operation that assumes that job!=null
>     JobInProgress job = jobs.get(jobid);
>     checkAccess(job, QueueManager.QueueOperation.ADMINISTER_JOBS);
> Recommend: add a test that calls this operaton with an invalid jobID, then fix the code as appropriate

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-4419) JobTracker.setJobPriority() doesn't check for a jobID lookup failing

Posted by "Hemanth Yamijala (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12639889#action_12639889 ] 

Hemanth Yamijala commented on HADOOP-4419:
------------------------------------------

One thing to consider is how to inform the client back about the failure. Because if we silently return, the operation is treated as successful, and a message is printed from the JobClient that the said job has been killed or it's priority changed. 

We could throw an exception (subclassing IOException), something like UnknownJobException, and handle it on the client. It doesn't seem right though to have such a class to extend IOException. Does it ?

Another option is to define a new API which returns a boolean to indicate success or failure of the operation. Something like:
{code}
boolean setJobPriority(JobID jobID, String priority);
{code}

Any other thoughts ?

> JobTracker.setJobPriority() doesn't check for a jobID lookup failing
> --------------------------------------------------------------------
>
>                 Key: HADOOP-4419
>                 URL: https://issues.apache.org/jira/browse/HADOOP-4419
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: mapred
>    Affects Versions: 0.19.0, 0.20.0
>            Reporter: Steve Loughran
>            Priority: Minor
>
> Looking at the entry points of the JobTracker API, it seems that JobTracker.setJobPriority()  doesnt expect the JobID lookup ever to return null
> It goes straight from lookup to checking access, an operation that assumes that job!=null
>     JobInProgress job = jobs.get(jobid);
>     checkAccess(job, QueueManager.QueueOperation.ADMINISTER_JOBS);
> Recommend: add a test that calls this operaton with an invalid jobID, then fix the code as appropriate

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HADOOP-4419) JobTracker.setJobPriority() doesn't check for a jobID lookup failing

Posted by "Steve Loughran (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12640110#action_12640110 ] 

Steve Loughran commented on HADOOP-4419:
----------------------------------------

-tools other than the CLI can call this IPC method; the checks should be in this call, rather than just hoping whoever wrote the client side code got it right. (i.e. I may not be as competent as others)

-I'm OK with an exception being thrown, because this is unusual behaviour, and it is an error to try and do this on an invalid job. If the method returns false, its up to the caller to guess why.

> JobTracker.setJobPriority() doesn't check for a jobID lookup failing
> --------------------------------------------------------------------
>
>                 Key: HADOOP-4419
>                 URL: https://issues.apache.org/jira/browse/HADOOP-4419
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: mapred
>    Affects Versions: 0.19.0, 0.20.0
>            Reporter: Steve Loughran
>            Priority: Minor
>
> Looking at the entry points of the JobTracker API, it seems that JobTracker.setJobPriority()  doesnt expect the JobID lookup ever to return null
> It goes straight from lookup to checking access, an operation that assumes that job!=null
>     JobInProgress job = jobs.get(jobid);
>     checkAccess(job, QueueManager.QueueOperation.ADMINISTER_JOBS);
> Recommend: add a test that calls this operaton with an invalid jobID, then fix the code as appropriate

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (HADOOP-4419) JobTracker.setJobPriority() doesn't check for a jobID lookup failing

Posted by "Chris Douglas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-4419?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Douglas resolved HADOOP-4419.
-----------------------------------

    Resolution: Duplicate

This is being fixed in HADOOP-4420

> JobTracker.setJobPriority() doesn't check for a jobID lookup failing
> --------------------------------------------------------------------
>
>                 Key: HADOOP-4419
>                 URL: https://issues.apache.org/jira/browse/HADOOP-4419
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: mapred
>    Affects Versions: 0.19.0, 0.20.0
>            Reporter: Steve Loughran
>            Priority: Minor
>
> Looking at the entry points of the JobTracker API, it seems that JobTracker.setJobPriority()  doesnt expect the JobID lookup ever to return null
> It goes straight from lookup to checking access, an operation that assumes that job!=null
>     JobInProgress job = jobs.get(jobid);
>     checkAccess(job, QueueManager.QueueOperation.ADMINISTER_JOBS);
> Recommend: add a test that calls this operaton with an invalid jobID, then fix the code as appropriate

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.