[
https://issues.apache.org/jira/browse/YARN-10003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17053197#comment-17053197
]
Szilard Nemeth commented on YARN-10003:
---------------------------------------
Hi [~bteke],
Thanks for working on this patch.
Some comments:
1. Regarding the changes in YarnConfigurationStore#checkVersion:
If I'm understanding the code correctly, it worked before like this:
getConfStoreVersion() was invoked to store the loaded version. If it was null,
getCurrentVersion() was invoked to get the version again. If this was
compatible with currentVersion, storeVersion() was called.
Now your code is different. I'm a bit concerned about this piece of code:
{code}
// when currentVersion is null, the checkVersion method is overridden
if (currentVersion.equals(loadedVersion)) {
return;
}
{code}
There's no guard for nullity of currentVersion. If it becomes null in any
circumstance, an NPE would be thrown. Can you please guard this case?
2. In the same method and in relation with point 1., I don't really get what
you wanted to say with this comment:
{code}
// when currentVersion is null, the checkVersion method is overridden
{code}
Could you clarify this a bit, please?
3. In TestLeveldbConfigurationStore#testIncompatibleVersion: Can you please add
a message to the assertEquals? Same applies to
TestZKConfigurationStore#testIncompatibleVersion?
4. Nit: In TestInMemoryConfigurationStore#checkVersion: The message should be
"checkVersion threw an exception" or "...has thrown an exception".
> YarnConfigurationStore#checkVersion throws exception that belongs to
> RMStateStore
> ---------------------------------------------------------------------------------
>
> Key: YARN-10003
> URL: https://issues.apache.org/jira/browse/YARN-10003
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Szilard Nemeth
> Assignee: Benjamin Teke
> Priority: Major
> Attachments: YARN-10003.001.patch, YARN-10003.002.patch,
> YARN-10003.003.patch, YARN-10003.004.patch
>
>
> RMStateVersionIncompatibleException is thrown from method "checkVersion".
> Moreover, there's a TODO here saying this method is copied from RMStateStore.
> We should revise this method a bit.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
