[
https://issues.apache.org/jira/browse/YARN-8018?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16408946#comment-16408946
]
Gour Saha commented on YARN-8018:
---------------------------------
[~csingh], thank you for the patch. I did a first pass review of the 003 patch.
Will need to spend little more time. But here are a few comments/suggestions to
start with -
h5. ClientAMService.java
1.
In method restart: say “Restart service” instead of “Start service” and also
log the current ugi user
2.
In method upgrade: log current ugi user
h5. ServiceContext.java
1.
Remove unnecessary change of adding import
h5. ServiceScheduler.java
1.
No benefit of java style doc for a private field. So you might want to switch
to block comment for -
{code:java}
/**
* This encapsulates the <code>app</code> with methods to upgrade the
* app.
*/
private ServiceManager serviceManager;
{code}
2.
In inner class ServiceEventHandler
{code:java}
.format("[SERVICE]: Error in handling event type {1}",
{code}
change \{1} to \{0}
h5. api.records.ComponentState.java
1.
add a semi-colon at the end of NEEDS_UPGRADE
h5. ServiceState.java
1.
Is the new state supposed to be called UPGRADE or UPGRADING?
h5. ServiceClient.java
1.
Put a period after the word “upgrade" in the below log line -
{code:java}
+ ". There is nothing to upgrade";
{code}
2.
h5. service.component.Component.java
1.
In inner class ComponentNeedsUpgradeTransition does the service state need to
be updated in the transition method?
h5. YarnServiceConstants.java
1.
{code:java}
String UPGRADES_DIR = "upgrades";
{code}
I would suggest to call it UPGRADE_DIR and set value to “upgrade”. See reason
below in CoreFileSystem.java.
h5. CoreFileSystem.java
1.
{code:java}
return new Path(getBaseApplicationPath(),
YarnServiceConstants.SERVICES_DIRECTORY + "/" +
YarnServiceConstants.UPGRADES_DIR + "/" + version);
{code}
This will be a problem if a user is trying to create a service named “upgrades”
since it collides with service-names under the SERVICES_DIRECTORY.
Unfortunately, the only way to avoid the unambiguity is to create something
like .yarn/upgrades/services/...
h5. UpgradeComponentsFinder.java
1.
{code:java}
"principle not supported by upgrade");
{code}
change principle to principal
h5. Package org.apache.hadoop.yarn.service.service
1.
For the new files ServiceEvent.java, etc. under the above package - should we
just create them under org.apache.hadoop.yarn.service rather than creating a
service.service sub package?
> Yarn service: Add support for initiating service upgrade
> --------------------------------------------------------
>
> Key: YARN-8018
> URL: https://issues.apache.org/jira/browse/YARN-8018
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Chandni Singh
> Assignee: Chandni Singh
> Priority: Major
> Attachments: YARN-8018.001.patch, YARN-8018.002.patch,
> YARN-8018.003.patch
>
>
> Add support for initiating service upgrade which includes the following main
> changes:
> # Service API to initiate upgrade
> # Persist service version on hdfs
> # Start the upgraded version of service
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
