[ 
https://issues.apache.org/jira/browse/MAPREDUCE-6690?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15414184#comment-15414184
 ] 

Jason Lowe commented on MAPREDUCE-6690:
---------------------------------------

Thanks for updating the patch!  Looks good overall with just a few nits:

I think the code would be cleaner if we leveraged 
Configuration#getStringCollection to get the conf values rather than checking 
for null and splitting on comma directly.  That method will return an empty 
collection if there are no values for the property, so then we can just remove 
some of the null checks and just loop over the items for each property.  Some 
of the null checks would change to !isEmpty checks to avoid doing unnecessary 
mkdirs, etc., during upload methods, but they could be completely removed in 
the limit checking code.

The totalConfigSize* variables are essentially loop-invariants, so they should 
be computed once in the Limits constructor rather than each addFile call.

Both TestMRJobs and TestJobResourceUploader assume any IOException is OK if the 
job submission is supposed to fail.  The unit tests should verify that the 
expected exception that failed the job submission was related to limits, 
otherwise we could be failing the job submission for the wrong reasons and the 
test would still pass.  I'm thinking something along the lines of checking the 
exception message for limits-related wording, but maybe there's a cleaner way.



> Limit the number of resources a single map reduce job can submit for 
> localization
> ---------------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-6690
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-6690
>             Project: Hadoop Map/Reduce
>          Issue Type: New Feature
>            Reporter: Chris Trezzo
>            Assignee: Chris Trezzo
>         Attachments: MAPREDUCE-6690-trunk-v1.patch, 
> MAPREDUCE-6690-trunk-v2.patch, MAPREDUCE-6690-trunk-v3.patch, 
> MAPREDUCE-6690-trunk-v4.patch, MAPREDUCE-6690-trunk-v5.patch, 
> MAPREDUCE-6690-trunk-v6.patch
>
>
> Users will sometimes submit a large amount of resources to be localized as 
> part of a single map reduce job. This can cause issues with YARN localization 
> that destabilize the cluster and potentially impact other user jobs. These 
> resources are specified via the files, libjars, archives and jobjar command 
> line arguments or directly through the configuration (i.e. distributed cache 
> api). The resources specified could be too large in multiple dimensions:
> # Total size
> # Number of files
> # Size of an individual resource (i.e. a large fat jar)
> We would like to encourage good behavior on the client side by having the 
> option of enforcing resource limits along the above dimensions.
> There should be a separate effort to enforce limits at the YARN layer on the 
> server side, but this jira is only covering the map reduce layer on the 
> client side. In practice, having these client side limits will get us a long 
> way towards preventing these localization anti-patterns.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to