[ 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)