[jira] [Commented] (YARN-8018) Yarn service: Add support for initiating service upgrade

[Gour Saha (JIRA)]

    [ 
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: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org