hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gour Saha (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-5610) Initial code for native services REST API
Date Mon, 10 Oct 2016 16:33:20 GMT

    [ 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


Mime
View raw message