[
https://issues.apache.org/jira/browse/YARN-353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13727448#comment-13727448
]
Hitesh Shah commented on YARN-353:
----------------------------------
First set of comments:
Minor style nits:
- Lines exceeding 80 chars in various places.
- indentation issues in yarn-default.xml
- ZK_STATE_STORE_PREFIX could be just RM_PREFIX + "zk.state-store." instead of
"rm-state-store"
- would it be preferable to not have a default value for the zk acl and enforce
the user to set it if zk state store is being used? Obviously, any additional
configuration headache but safer in forcing users to set it explicitly.
- why are some settings commented out in yarn-default.xml and some aren't?
Also, the documented defaults do not seem to match the default values used in
code.
- from hadoop-yarn-server-resourcemanager/pom.xml, versions of zookeeper should
not be defined here. Use dependency management section defined in
hadoop-project/pom.xml to have a consistent version across all components in
hadoop.
ZKRMStateStore comments:
- "throw new YarnRuntimeException("No ZK_RM_STATE_STORE_ADDRESS specified");"
is not useful as it does not clarify the actual property name that was not
specified.
- How is using a default of "world:anyone:rwcda" different from the fallback
usage to the acls defined by "Ids.OPEN_ACL_UNSAFE"? If an invalid acl is
defined, does it get treated as an error or fallback to open acls? Unit test
for this maybe?
- have not looked at the zk apis in detail, but why are we setting a watch
using exists() before calling delete()? Also, return value of exists() is being
ignored and delete called regardless?
- should there be some versioning info that should be stored with the data for
future versions to understand how to deserialize data?
- even though we are using PB which will handle addition of new fields, it
might be interesting to understand what version of the software wrote the data
and use that info to understand whether some form of defaults need to be
augmented or translations to be done, etc. To summarize, do we require a
version id in the payload?
- Is there any area that you see which may create problems when handling either
upgrades or downgrades of software?
- "Unknown child node with name" - should we throw an error instead or ignore
unknowns?
- Is it really necessary to have "LOG.info("Storing info for " to be at the
INFO level? DEBUG maybe?
- the version info from getData() seems to be unused for future setData calls?
It seems like setData is only called from the unit tests?
- rmstatestore and zk store both seem to be catching and logging the same
exception before throwing it back down the chain. e.g Error storing info for
attempt and Error storing appAttempt
> Add Zookeeper-based store implementation for RMStateStore
> ---------------------------------------------------------
>
> Key: YARN-353
> URL: https://issues.apache.org/jira/browse/YARN-353
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Hitesh Shah
> Assignee: Bikas Saha
> Attachments: YARN-353.1.patch, YARN-353.2.patch, YARN-353.3.patch,
> YARN-353.4.patch, YARN-353.5.patch, YARN-353.6.patch, YARN-353.7.patch,
> YARN-353.8.patch, YARN-353.9.patch
>
>
> Add store that write RM state data to ZK
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
