[
https://issues.apache.org/jira/browse/YARN-353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13734473#comment-13734473
]
Karthik Kambatla commented on YARN-353:
---------------------------------------
bq. Identation still off in yarn-default.xml
bq. hadoop-yarn-server-resourcemanager/pom.xml is still not fixed - contains
the version info.
LOG.debug() should be wrapped within "if LOG.isDebugEnabled() {}"
bq. In "abstract class ZKAction<T>", member variables should be final.
bq. findbugs issue - zkSessionTimeout does not seem to be an issue but zkClient
is something which is read/modified multiple times in various functions. Which
functions are the ones that are/cannot be synchronized that access zkClient?
bq. "os.close();" - close calls should be in a finally block ( in multiple
places ).
bq. "LOG.info("Error in storing " + dtSequenceNumberPath);" - change to
LOG.info("Error in storing " + dtSequenceNumberPath, e);"
bq. "LOG.info("Created new connection for " + this);" --> logging 'this'? is
there a toString() function?
bq. How is a connection failure to zk handled? i.e. getNewZooKeeper() throws an
exception. Does the RM fail/shutdown? Is the connection retried at a later
point?
bq. what if the first line throws an exception saying node exists but the other
nodes are not created? Shouldn't each call be in its own try catch block? Or
should the create function be changed to accept a parameter which when set
causes the function to ignore node exists errors?
bq. For deleteWithRetries, the return code of exists() could be checked if a
delete is required or not.
Fixed.
bq. When creating a Zookeeper object, ZK apis support a base root path and all
operations are done relative to the base root path? Any reason why we are not
using that approach by initializing zk with zkRootNodePath ?
We are calling createWithRetries on multiple roots
bq. why the catch and re-throw? ( in multiple places )
The HA code might want to deal with those scenarios differently. Given we are
on the verge of implement ZK-based HA, I think it is okay to leave these as
they currently are.
bq. validation only for non-null and not a valid format?
While it is a good idea to check for a valid format, the current behavior is
how RM etc. deal with host:port arguments
bq. Not sure if a default value of "<!-value>127.0.0.1:2181</value->" should be
mentioned in yarn-default.xml.
Changed the comment to be host:port. Leaving it there makes it easier for users.
> 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.10.patch, YARN-353.11.patch, YARN-353.12.patch,
> yarn-353-12-wip.patch, 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
