[jira] [Commented] (YARN-2404) Remove ApplicationAttemptState and ApplicationState class in RMStateStore class

["Jian He (JIRA)" <ji...@apache.org>]

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

Jian He commented on YARN-2404:
-------------------------------

Tsuyoshi, thanks for your effort on this issue ! 
I looked at  patch YARN-2404.1.patch, looks good overall, some minor comments:
- FileSystemRMStateStore: how about changing appId parameter to use ApplicationId instead of String.
{code}
  private Path getAppDir(Path root, String appId) {
    return getNodePath(root, appId);
  }
{code}
- log the exception here as well ?
{code}
    try {
      attemptState =
          ApplicationAttemptStateData.newInstance(
              appAttempt.getAppAttemptId(),
              appAttempt.getMasterContainer(), ApplicationAttemptStateData
                  .convertCredentialsToByteBuffer(credentials),
              appAttempt.getStartTime());
    } catch (IOException ioe) {
      notifyStoreOperationFailed(ioe);
    }
{code}
- In RMStateStore#removeApplication, maybe just pass null for the attemptState, as the attemptState is not used anyways.
{code}
      appState.attempts.put(attemptState.getAttemptId(), attemptState);
    }
{code}
- TestRMRestart#convertCredentailsFromByteBuffer, move this to ApplicationAttemptStateData; And make RMAppAttemptImpl/RMStateStoreTestBase use this method also.
{code}
    if (appAttemptTokens.hasArray()) {
      DataInputByteBuffer dibb = new DataInputByteBuffer();
      credentials = new Credentials();
      appAttemptTokens.rewind();
      dibb.reset(appAttemptTokens);
      credentials.readTokenStorageStream(dibb);
    }
{code}

> Remove ApplicationAttemptState and ApplicationState class in RMStateStore class 
> --------------------------------------------------------------------------------
>
>                 Key: YARN-2404
>                 URL: https://issues.apache.org/jira/browse/YARN-2404
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Jian He
>            Assignee: Tsuyoshi OZAWA
>         Attachments: YARN-2404.1.patch, YARN-2404.2.patch
>
>
> We can remove ApplicationState and ApplicationAttemptState class in RMStateStore, given that we already have ApplicationStateData and ApplicationAttemptStateData records. we may just replace ApplicationState with ApplicationStateData, similarly for ApplicationAttemptState.



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