[
https://issues.apache.org/jira/browse/MAPREDUCE-1376?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12853572#action_12853572
]
Chris Douglas commented on MAPREDUCE-1376:
------------------------------------------
bq. I feel , it is clean . I have added more code for creation of staging
directories in it so I thought TestGridmixSubmission was getting really big
with some reusable components . I think we might use those components again in
future.
Code written with {{\*Util}} classes tends to be difficult to read and the
tenuous dependencies between tests created through these classes are avoidable.
Concerning its content, other than boilerplate {{MiniMRCluster}} code (which
must be called in the same, boilerplate way from the test) and the one-line
{{changePermission}}, there's only {{createHomeAndStagingDirectory}}, which
calls back to the {{TestGridmixSubmission}} {{LOG}} field, anyway. But OK.
bq. parseUserList is only being used by RoundRobinUserResolve to iam leaving
the interface as it is.
Abstract classes are generally easier to evolve than interfaces- and creating
new {{UserResolvers}} without rewriting the parser or depending on
{{RoundRobinUserResolver}} is a pleasant property- but OK. Making
{{parseUserList}} protected would at least allow subclasses of RRUR to share
the parsing code.
* There is some commented-out code in {{TestGridmixSubmission}}:
{noformat}+//
GridmixTestUtils.createHomeAndStagingDirectory((JobConf)conf);{noformat}
* There is a javadoc error since {{parseUserList}} is moved:
{noformat}+ * listing target users. The format of this file is defined by
{...@link
+ * #parseUserList}.{noformat}
* Many javadoc comments seem to include {{</p>}} at random
* Many of the exception and log messages start with spaces, e.g. {{" Could not
run job submitted "}}, which is not conventional. Please don't do this. There
are also many spelling errors in the messages, e.g. {{LOG.error(" EXCEPTOIN in
availableSlots ", e);}}
* Should jobs be submitted with ACLs so that the statistics can be viewed by
the submitting user? This would allow statistics, etc. to be collected without
requiring a {{doAs}} block, right?
* {{GridmixJob::call}} can declare the exceptions it will throw instead of
catching everything and returning null. This is true of all the
{{PrivilegedExceptionAction}} uses, including the {{doAs}} calling
{{Gridmix::runJob}}, which can have a signature narrower than {{Exception}}
* {{TestGridmixSubmission::doSubmission}} should take a
{{GridmixJobSubmissionPolicy}} as an argument, rather than setting a static and
calling the method from each submission type.
+1 Overall. None of these need to block commit, but please attend to them as
part of either the Apache patch or concurrently with related work.
> Support for varied user submission in Gridmix
> ---------------------------------------------
>
> Key: MAPREDUCE-1376
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-1376
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: contrib/gridmix
> Reporter: Chris Douglas
> Assignee: Chris Douglas
> Attachments: 1376-2-yhadoop-security.patch,
> 1376-3-yhadoop20.100.patch, 1376-4-yhadoop20.100.patch,
> 1376-5-yhadoop20-100.patch, 1376-yhadoop-security.patch, M1376-0.patch,
> M1376-1.patch, M1376-2.patch, M1376-3.patch, M1376-4.patch
>
>
> Gridmix currently submits all synthetic jobs as the client user. It should be
> possible to map users in the trace to a set of users appropriate for the
> target cluster.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
