[
https://issues.apache.org/jira/browse/YARN-8048?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16429280#comment-16429280
]
Gour Saha commented on YARN-8048:
---------------------------------
[~rohithsharma] thank you for the patch. Sorry for the late review. This is a
very interesting and helpful feature and the patch looks great. Few minor
comments which can be taken care of in an addendum patch of maybe another
sub-task under the parent (if they make sense to you) -
1. SystemServiceManagerImpl.java
{code:java}
if (!services.add(service)) {
int count = ignoredUserServices.containsKey(userName) ?
ignoredUserServices.get(userName) : 0;
ignoredUserServices.put(userName, count + 1);
LOG.warn(
"Ignoring service {} for the user {} as it is already present,"
+ " filename = {}", service.getName(), userName, filename);
}
LOG.info("Added service {} for the user {}, filename = {}",
service.getName(), userName, filename);
{code}
I think the info log will get printed here every time the warn log is also
printed inside the if block. Should the info log go in the else block?
2. Should we rename TestSystemServiceImpl.java to
TestSystemServiceManagerImpl.java?
3. TestSystemServiceImpl
{code:java}
Assert.assertTrue(
"Service name doesn't exist in expected " + "userService "
+ serviceNames, serviceNames.contains(next.getName()));
{code}
Should we combine these 2 strings into 1 -> "Service name doesn't exist in
expected " + "userService "
4. Do we need to add documentation about "yarn.service.system-service.dir”
anywhere?
5. Under
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources/
should we rename the dir “users" to "system-services" to better reflect what
these files/tests are for (and change resourcePath in
TestSystemServiceImpl.java accordingly)
6. For an additional test to see if a dir is skipped, do you want to add a
directory named “bad" (probably needs a dummy file under it otherwise github
will not allow you to commit an empty dir) under the path
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services-api/src/test/resources//users/sync/user1/?
> Support auto-spawning of admin configured services during bootstrap of
> rm/apiserver
> -----------------------------------------------------------------------------------
>
> Key: YARN-8048
> URL: https://issues.apache.org/jira/browse/YARN-8048
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Rohith Sharma K S
> Assignee: Rohith Sharma K S
> Priority: Major
> Fix For: 3.2.0
>
> Attachments: YARN-8048.001.patch, YARN-8048.002.patch,
> YARN-8048.003.patch, YARN-8048.004.patch, YARN-8048.005.patch,
> YARN-8048.006.patch
>
>
> Goal is to support auto-spawning of admin configured services during
> bootstrap of resourcemanager/apiserver.
> *Requirement:* Some of the services might required to be consumed by yarn
> itself ex: Hbase for atsv2. Instead of depending on user installed HBase or
> sometimes user may not required to install HBase at all, in such conditions
> running HBase app on YARN will help for ATSv2.
> Before YARN cluster is started, admin configure these services spec and place
> it in common location in HDFS. At the time of RM/apiServer bootstrap, these
> services will be submitted.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
