[ https://issues.apache.org/jira/browse/YARN-353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13697629#comment-13697629 ]
Devaraj K commented on YARN-353: -------------------------------- The patch overall looks good, here are my observations on the patch. 1. {code:xml} + <property> + <description>ACL's to be used for ZooKeeper znodes. + This may be supplied when using + org.apache.hadoop.yarn.server.resourcemanager.recovery.ZKRMStateStore + as the value for yarn.resourcemanager.store.class</description> + <name>yarn.resourcemanager.zk.rm-state-store.timeout.ms</name> + <!--value>world:anyone:rwcda</value--> + </property> {code} Here configuration name should be "yarn.resourcemanager.zk.rm-state-store.acl". 2. {code:xml} + // protected to mock for testing + protected synchronized ZooKeeper getNewZooKeeper() throws Exception { {code} Can we also annotate with @VisibleForTesting for this method? 3. {code:xml} + /** HostPort of ZK server for ZKRMStateStore */ + <description>HostPort of the ZooKeeper server when using {code} These two places can we use Host:Port instead of HostPort for comment/description. 4. {code:xml} + zkHostPort = conf.get(YarnConfiguration.ZK_RM_STATE_STORE_ADDRESS); {code} Can we use the default value for this config with this as present for other props, {code:xml} + <!--value>127.0.0.1:2181</value--> {code} 5. {code:xml} + public static final String DEFAULT_ZK_RM_STATE_STORE_PARENT_PATH = ""; {code} Can we use the default value for this config with this instead of having empty, {code:xml} + <!--value>/rmstore</value--> {code} > 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 > > > 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