hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Douglas (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MAPREDUCE-1376) Support for varied user submission in Gridmix
Date Mon, 05 Apr 2010 22:34:27 GMT

    [ 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.


Mime
View raw message