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.
  private Path getAppDir(Path root, String appId) {
    return getNodePath(root, appId);
- log the exception here as well ?
    try {
      attemptState =
              appAttempt.getMasterContainer(), ApplicationAttemptStateData
    } catch (IOException ioe) {
- In RMStateStore#removeApplication, maybe just pass null for the attemptState, 
as the attemptState is not used anyways.
      appState.attempts.put(attemptState.getAttemptId(), attemptState);
- TestRMRestart#convertCredentailsFromByteBuffer, move this to 
ApplicationAttemptStateData; And make RMAppAttemptImpl/RMStateStoreTestBase use 
this method also.
    if (appAttemptTokens.hasArray()) {
      DataInputByteBuffer dibb = new DataInputByteBuffer();
      credentials = new Credentials();

> 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

Reply via email to