[jira] [Commented] (TEZ-692) Unify job submission in either TezClient or TezSession

["Hitesh Shah (JIRA)" <ji...@apache.org>]

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

Hitesh Shah commented on TEZ-692:
---------------------------------

Comments:
  
- Separate jira to address hardcoded value of SLEEP_FOR_READY and other magic numbers?

{code}
   * @param name Name of the client
{code}
  -  How is this used in session vs non-session mode? Does the doc need to change for more clarity. Also, if a user uses TezClient to launch one and only one DAG. is the name needed?

{code}
    this.amConfig = new AMConfiguration(tezConf);
    if (localResources != null) {
      this.amConfig.addLocalResources(localResources);
    }
    if (credentials != null) {
      this.amConfig.setCredentials(credentials);
    }
{code}
   - maybe just change the amConfig constructor to accept local resources and creds. Checks should likely be done inside the amconfig class.

bq. addAppMasterLocalResources()
  - some of the doc is incorrect with respect to non-session mode. Adding new resources for a session are restricted to file due to classpath loading issues. For non-session mode, this need not hold true. Likewise for the private visibility. 

bq.         LOG.warn("DAG submission to session timed out, stopping session");
  - should probably log client timeout value

{code}
LOG.info("Submitted dag to TezSession"
        + ", sessionName=" + sessionName
        + ", applicationId=" + sessionAppId
        + ", dagId=" + dagId);
{code}
  - logging the dag name would be useful here. 

{code}
    if (yarnClient != null) {
      yarnClient.close();
    }
{code}
   - should likely be in a finally block and any exceptions thrown by this should be logged and ignored. 

bq. getSessionApplicationId
  - why is the function name session specific? 

bq. getSessionName
  - unused function. Also should it be session specific? The TezClient ctor takes a name so could just rename to getName().

bq. clearLocalResource
  - unused function. What happens to a session AM's classpath when this is invoked?

bq. pre-warm has hardcoded sleep for 1000ms. 
  - Could be combined with other hardcoded values into a common poll interval. 



 



 

> Unify job submission in either TezClient or TezSession
> ------------------------------------------------------
>
>                 Key: TEZ-692
>                 URL: https://issues.apache.org/jira/browse/TEZ-692
>             Project: Apache Tez
>          Issue Type: Sub-task
>            Reporter: Bikas Saha
>            Assignee: Bikas Saha
>         Attachments: TEZ-692.1.patch, TEZ-692.2.patch, TEZ-692.3.patch, TEZ-692.4.patch
>
>
> Its confusing to have 2 ways to create and submit a tez job. The developer has to spend time thinking about and deciding which method to use.



--
This message was sent by Atlassian JIRA
(v6.2#6252)