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