[
https://issues.apache.org/jira/browse/MAPREDUCE-2863?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13158991#comment-13158991
]
Hitesh Shah commented on MAPREDUCE-2863:
----------------------------------------
Browsed through the patch. The outputs look good. More consistent between the 2
formats now. I haven't run a cluster with the patch yet - still need to try
that.
Thomas, a quick question on the output format. Is that only controlled by the
incoming accept header? Is there a default to which the framework defaults to
if the accept header is set to */*? Is there a way to control the output format
via a query parameter?
General comment about the info objects:
- does it make sense to declare most of the member variables of the *Info
classes to be final?
- there are a few places where there is commented out code left around.
Comments:
NMWebServices#getNodeApps
{code}
+ if (appInfo != null) {
+ allApps.add(appInfo);
+ }
{code}
- Is the null check required? Likewise for other cases where an info object is
initialized via its constructor.
TestRMWebServices#verifyClusterInfoXML
- remove dead code - NodeList nl = docEle.getElementsByTagName("Employee");
- the test needs to be enhanced to match the json checks.
- using any converter api from xml to json or vice versa, a single content
verification function can be used to verify both outputs as well as check
consistency in elements across the two formats.
TestRMWebServices
- matching the version info - this can potentially also be done by getting the
version from the environment by having a system property be set. See pom for
yarn.applications.distributedshell or mapreduce-jobclient.
RMWebServices
- minor comment - s/UriInfo uriinfo/UriInfo uriInfo/;
- same comment about null checks as above.
RMWebServices#hasAccess
- my knowledge on hadoop security is still limited but do we need to worry
about remoteUser or callerugi being null in a secure setup? If yes, a null
value should make hasAccess return false in such a setup.
RMWebServices#getUserMetricsInfo
- remove commented out code if not needed?
RMWebServices#getNodeInfo
- with reference to 'nodeId.split(":")' , you may need to confirm that all
query params get url decoded automatically.
RMWebServices#getNodes
- for state, should we check for invalid value being passed in? It will work
correctly as it will not return any objects but a question from the point of
view of a spec as to whether we should strictly check incoming query params for
correctness/validity?
RMWebServices#getApps
- check for invalid values? count=0/negative time vals/fEnd > fBegin?
- minor optimization - s/app.getStartTime() < sBegin || app.getStartTime() >
sEnd/app.getStartTime() >= sBegin && app.getStartTime <= sEnd/
- remove commented out code.
- the user and queue name seems to be compared without case sensitivity. Is
this correct?
- inconsistency in handling of secure access. getApps hides away some
information if certain apps are not accessible whereas getApp throws an
exception.
DefaultSchedulerInfo
- Could we just do this.qstate = qInfo.getQueueState().name ?
- should this be called FifoSchedulerInfo instead of Default? It gives an
assumption that fifo will always be the default.
NodeInfo
{code}
+ numContainers = report.getNumContainers();
+ usedMemory = report.getUsedResource().getMemory();
+ availableMemory = report.getAvailableResource().getMemory();
{code}
- Not sure why we need local scope vars to copy from report before setting
the class's member variables.
RM/Webapp/AppInfo
- some unused commented out code in ctor that could be removed.
JobHistoryParser
- Not sure about legacy tools but would something like changing default val
of errorInfo in jobhistory from "None" to "" affect things?
(Job)HsWebServices:
- @Path("/ws/v1/history")
- should this be change to be more specific to denote MR job history?
- secure access? This does not seem to enforce any checks. Again, just a
question to bridge my knowledge gap.
HsWebServices#getJobs
- similar comments as above regd checks on invalid params, case sensitive
comparisons
AMWebServices#getJobs
- Inconsistent data across getJobs and getJob for secure access.
AMWebServices#getJobTaskAttemptId
- This function and a couple of others return a null when data is not found
as compared to NotFoundExceptions in other cases. Is null handled as a 404 by
the framework similar to NotFoundException ?
{code}
+ for (TaskAttempt attempt : attempts.values()) {
+
+ int newAttempts = 0, running = 0, successful = 0, failed = 0, killed =
0;
{code}
- not sure how much of an issue this creates with the java gc but would it be
better to declare once outside the for loop and reset on each loop? Saw this in
a couple of other places.
> Support web-services for RM & NM
> --------------------------------
>
> Key: MAPREDUCE-2863
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2863
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: mrv2, nodemanager, resourcemanager
> Affects Versions: 0.23.0
> Reporter: Arun C Murthy
> Assignee: Thomas Graves
> Priority: Blocker
> Attachments: MAPREDUCE-2863.patch, MAPREDUCE-2863.patch,
> MAPREDUCE-2863.patch, amoutput.txt, appoutput.txt, hsoutput.txt,
> nmoutput.txt, nmoutput.txt, nmoutput.txt, rmoutput.txt, rmoutput.txt,
> rmoutput.txt
>
>
> It will be very useful for RM and NM to support web-services to export
> json/xml.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
