Return-Path: <yarn-issues-return-97505-archive-asf-public=cust-asf.ponee.io@hadoop.apache.org>
X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io
Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io
Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183])
	by cust-asf2.ponee.io (Postfix) with ESMTP id A9233200B9C
	for <archive-asf-public-internal@cust-asf2.ponee.io>; Mon, 10 Oct 2016 18:33:22 +0200 (CEST)
Received: by cust-asf.ponee.io (Postfix)
	id A7CD2160AE1; Mon, 10 Oct 2016 16:33:22 +0000 (UTC)
Delivered-To: archive-asf-public@cust-asf.ponee.io
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by cust-asf.ponee.io (Postfix) with SMTP id BD735160AD1
	for <archive-asf-public@cust-asf.ponee.io>; Mon, 10 Oct 2016 18:33:21 +0200 (CEST)
Received: (qmail 83267 invoked by uid 500); 10 Oct 2016 16:33:20 -0000
Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:yarn-issues-help@hadoop.apache.org>
List-Unsubscribe: <mailto:yarn-issues-unsubscribe@hadoop.apache.org>
List-Post: <mailto:yarn-issues@hadoop.apache.org>
List-Id: <yarn-issues.hadoop.apache.org>
Delivered-To: mailing list yarn-issues@hadoop.apache.org
Received: (qmail 83247 invoked by uid 99); 10 Oct 2016 16:33:20 -0000
Received: from arcas.apache.org (HELO arcas) (140.211.11.28)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Oct 2016 16:33:20 +0000
Received: from arcas.apache.org (localhost [127.0.0.1])
	by arcas (Postfix) with ESMTP id 8FB602C0D52
	for <yarn-issues@hadoop.apache.org>; Mon, 10 Oct 2016 16:33:20 +0000 (UTC)
Date: Mon, 10 Oct 2016 16:33:20 +0000 (UTC)
From: "Gour Saha (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.13002107.1472746424000.783721.1476117200585@Atlassian.JIRA>
In-Reply-To: <JIRA.13002107.1472746424000@Atlassian.JIRA>
References: <JIRA.13002107.1472746424000@Atlassian.JIRA> <JIRA.13002107.1472746424167@arcas>
Subject: [jira] [Commented] (YARN-5610) Initial code for native services
 REST API
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394
archived-at: Mon, 10 Oct 2016 16:33:22 -0000


    [ https://issues.apache.org/jira/browse/YARN-5610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15562742#comment-15562742 ] 

Gour Saha commented on YARN-5610:
---------------------------------

{quote}
The component name is used in the compNameArtifactIdMap but then the name is overridden by the artifact Id, is the component name expected to be overridden? the API document says this field is mandatory
this
{noformat}
          compNameArtifactIdMap.put(comp.getName(), comp.getArtifact().getId());
          comp.setName(comp.getArtifact().getId());
        }
{noformat}
{quote}
Yes, the idea is to create a mapping of all component names to artifact ids for components of type external APPLICATION. App-owners will specify dependencies for a complex app by referring to the (arbitrarily chosen) component names. However for external apps, the component name does not signify anything. It is the artifact id which Slider internally cares about and actually exists in a well known registry. 

{quote}
comments seems not matching code
{noformat}
    // If artifact is of type APPLICATION, then in the POST JSON there will
    // be no component definition for that artifact. Hence it's corresponding id
    // field is added. Every external APPLICATION has a unique id field.
    List<String> convertedDeps = new ArrayList<>();
    for (String dep : component.getDependencies()) {
      if (compNameArtifactIdMap.containsKey(dep)) {
        convertedDeps.add(compNameArtifactIdMap.get(dep));
      } else {
        convertedDeps.add(dep);
      }
    }
{noformat}
{quote}
This comment is related to the previous point about why compNameArtifactIdMap is built in the first place for external APPLICATIONs. Can you explain a little more why you think the comment does not match the code?

{quote}
I have some TODOs on global config which I wanted to get back to. Let me check and remove if not required.
I don't see the TODOs in this method, should it be removed ? similarly configPrefix
{quote}
Here is the TODO line for globalConf -
{noformat}
    // TODO: add it to yaml
    Configuration globalConfig = null;
    //    Configuration globalConfig = (Configuration) SerializationUtils
    //        .clone(application.getConfiguration());
{noformat}
Let me talk to [~billie.rinaldi] on globalConf and configPrefix, and then either remove or restore the code.

{quote}
why is queueName set to label_expression ? Also queueName is not being set anywhere
{noformat}
    if (queueName != null) {
      resCompOptTriples.addAll(Arrays.asList(compName,
          ResourceKeys.YARN_LABEL_EXPRESSION, queueName));
    }
{noformat}
{quote}
SliderClient reads queue name from YARN_LABEL_EXPRESSION and then sets it in the ApplicationSubmissionContext (in AppMasterLauncher.java) -
{code}
submissionContext.setNodeLabelExpression(extractLabelExpression(options));
{code}
Also the 003 patch has the following line to set queueName -
{code}
    final String queueName = application.getQueue();
{code}

{quote}
why is the PROPERTY_APP_RUNAS_USER variable needed? Usually we switch to the correct user and then start the service. Also, it's not appropriate to run it as root if user is not set. IIUC, I think setting "SLIDER_USER = UserGroupInformation.getCurrentUser();" is enough.
{noformat}
  private static String getUserToRunAs() {
    String user = System.getenv(PROPERTY_APP_RUNAS_USER);
    if (StringUtils.isEmpty(user)) {
      user = "root";
    }
    return user;
  }
{noformat}
{quote}
This is to create a hook from the standalone script _*run_rest_service.sh*_ to be able to bring up the rest service as any desired user. This can be removed after we make the decision to start this as part of the RM process or otherwise, and as you rightly mentioned change it to UserGroupInformation.getCurrentUser().

{quote}
sorry, didn't get it. why you need to bind "help", when creating slider client ?
If you see ActionHelpArgs you will see that it overrides getHadoopServicesRequired and returns false. Help is the safest action to bind since there are hardly any other action which does not need any additional params
{quote}
SliderClient currently requires you to bind to some argument before initializing. It is probably not required and should be cleaned up in slider-core code, but until then if I don't call SliderClient.bindArgs then SliderClient.serviceInit will throw NPE. I will file a Slider bug to remove this dependency and then we can cleanup api service code as well.

{quote}
why here in deleteApplication, it needs to wait for the appName to appear? I'm afraid such short sleep interval with a getAllApplcations call will overwhelm RM if any bug appears and the loop doesn't break.
{noformat}
      while (getSliderList(appName) == 0) {
        Thread.sleep(100); // don't use thread sleep
      }
{noformat}
{quote}
Slider destroy fails until stop has completed successfully. I improved the code to make the loop have a finite upper bound (5 for now) and the sleep to be 500ms. So max 2.5 secs sleep and max 5 calls to RM.
{code}
    // Although slider client stop returns immediately, it usually takes a
    // little longer for it to stop from YARN point of view. Slider destroy
    // fails if the application is not completely stopped. Hence the need to
    // call destroy in a controlled loop few times (only if exit code is
    // EXIT_APPLICATION_IN_USE), before giving up.
    boolean keepTrying = true;
    int maxDeleteAttempt = 5;
    int deleteAttempt = 0;
    while (keepTrying && deleteAttempt < maxDeleteAttempt) {
      try {
        destroySliderApplication(appName);
        keepTrying = false;
      } catch (SliderException e) {
        logger.error("Delete application threw exception", e);
        if (e.getExitCode() == SliderExitCodes.EXIT_APPLICATION_IN_USE) {
          deleteAttempt++;
          try {
            Thread.sleep(500);
          } catch (InterruptedException e1) {
          }
        } else {
          return Response.status(Status.INTERNAL_SERVER_ERROR).build();
        }
      } catch (Exception e) {
        logger.error("Delete application failed", e);
        return Response.status(Status.INTERNAL_SERVER_ERROR).build();
      }
    }
{code}


> Initial code for native services REST API
> -----------------------------------------
>
>                 Key: YARN-5610
>                 URL: https://issues.apache.org/jira/browse/YARN-5610
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Gour Saha
>            Assignee: Gour Saha
>             Fix For: yarn-native-services
>
>         Attachments: YARN-4793-yarn-native-services.001.patch, YARN-5610-yarn-native-services.002.patch, YARN-5610-yarn-native-services.003.patch
>
>
> This task will be used to submit and review patches for the initial code drop for the native services REST API 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org