[
https://issues.apache.org/jira/browse/MAPREDUCE-5870?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14735105#comment-14735105
]
Jason Lowe commented on MAPREDUCE-5870:
---------------------------------------
Thanks for updating the patch!
We cannot change ClientProtocol.setJobPriority to take an integer rather than a
String. That's a backwards-incompatible change.
Why are we adding ClientProtocol.getJobPriority? We are already returning the
job priority via getJobStatus, so I'm a little confused on why we are extending
the protocol to get information the client can already retrieve.
Does it make more sense to have LocalJobRunner.getJobPriority return a normal
or default priority? Returning null seems disruptive, as priorities really
don't matter in the local job mode scenario. In a similar sense, I'm wondering
if we should have LocalJobRunner.setJobPriority simply track or ignore the
priority rather than explode. Since it already exploded for setJobPriority
before we can tackle that in a separate JIRA if desired.
Why does JobConf.setPriorityAsInteger take a boxed type rather than a normal
int and call Integer.toString?
I think the new TypeConverter functions are a little too generically named.
It's a bit dangerous to have a toYarn that takes something as generic as a
String and returns an int -- it could easily me misused in other contexts that
have nothing to do with job priorities. I'd name these fromYarnPriority,
toYarnPriority, etc. so there's no chance of accidental collision.
convertPriorityToInteger needs to handle the case where the string is itself an
integer, otherwise setJobPriorityAsInteger followed by getJobPriorityAsInteger
doesn't work as expected. Trying to do JobPriority.valueOf on a string that
may not contain a valid enum is going to throw an IllegalArgumentException.
Please add unit tests for the various get/set as integer scenarios on both Job
and JobConf to make sure we're handling them properly.
Why was the call to testChangingJobPriority removed from TestMRJobClient? The
function is still there, although in practice is never called.
> Support for passing Job priority through Application Submission Context in
> Mapreduce Side
> -----------------------------------------------------------------------------------------
>
> Key: MAPREDUCE-5870
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-5870
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: client
> Reporter: Sunil G
> Assignee: Sunil G
> Attachments: 0001-MAPREDUCE-5870.patch, 0002-MAPREDUCE-5870.patch,
> 0003-MAPREDUCE-5870.patch, 0004-MAPREDUCE-5870.patch,
> 0005-MAPREDUCE-5870.patch, 0006-MAPREDUCE-5870.patch, Yarn-2002.1.patch
>
>
> Job Prioirty can be set from client side as below [Configuration and api].
> a. JobConf.getJobPriority() and
> Job.setPriority(JobPriority priority)
> b. We can also use configuration
> "mapreduce.job.priority".
> Now this Job priority can be passed in Application Submission
> context from Client side.
> Here we can reuse the MRJobConfig.PRIORITY configuration.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
