hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Allen Wittenauer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-3484) Fix up yarn top shell code
Date Sun, 26 Apr 2015 17:09:39 GMT

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

Allen Wittenauer commented on YARN-3484:
----------------------------------------

* variables that are local to a function should be declared local.  
* avoid using mixed case as per the shell programming guidelines
* yarnTopArgs is effectively a global.  It should either get renamed to YARN_foo or another
 to not pollute the shell name space or another approach is process set_yarn_top_args as a
subshell, reading its input directly to avoid the global entirely
* set_yarn_top_args should be hadoop_ something so as to not pollute the shell name space

* nit: technically, TERM isn't guaranteed to be set on all OSes under all workable modes,
since it is the login process' responsibility to set it.  However, almost all modern systems
do set it and it's fairly reliable. I think it's OK to leave the check, but I wanted to make
this comment here for future readers in case they hit the situation where TERM wasn't set
for their particular system.  Yes, that situation was thought about, but honestly, upgrade.

> Fix up yarn top shell code
> --------------------------
>
>                 Key: YARN-3484
>                 URL: https://issues.apache.org/jira/browse/YARN-3484
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: scripts
>    Affects Versions: 3.0.0
>            Reporter: Allen Wittenauer
>            Assignee: Varun Vasudev
>         Attachments: YARN-3484.001.patch
>
>
> We need to do some work on yarn top's shell code.
> a) Just checking for TERM isn't good enough.  We really need to check the return on tput,
especially since the output will not be a number but an error string which will likely blow
up the java code in horrible ways.
> b) All the single bracket tests should be double brackets to force the bash built-in.
> c) I'd think I'd rather see the shell portion in a function since it's rather large.
 This will allow for args, etc, to get local'ized and clean up the case statement.



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

Mime
View raw message