[
https://issues.apache.org/jira/browse/YARN-353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13729050#comment-13729050
]
Jian He commented on YARN-353:
------------------------------
bq. Lines exceeding 80 chars in various places.
bq. indentation issues in yarn-default.xml
bq. from hadoop-yarn-server-resourcemanager/pom.xml, versions of zookeeper
should not be defined here.
fixed
bq.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.
the only user now reads and writes to zk state store is RM itself. in some
sense we may trust RM. As you said, this may create more burden on user to
configure.
bq.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.
Fixed. except yarn.resourcemanager.zk.state-store.address, which is expected
for user to configure explicitly.
bq. "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.
fixed
bq. 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?
added a test case to test invalid acl and should catch exception.
bq. 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?
the exists() call is only for the purpose of setting a watch, so that the
following delete call will trigger the watch. In zookeeper , watch is one-time
trigger. if the watch is already triggered, the watch event will not be sent
unless the client sets the watch again
bq. should there be some versioning info that should be stored with the data
for future versions to understand how to deserialize data? To summarize, do we
require a version id in the payload?
For now, each app or attempt info only have one version
bq. Is there any area that you see which may create problems when handling
either upgrades or downgrades of software?
may not support downgrade..
bq. "Unknown child node with name" - should we throw an error instead or ignore
unknowns?
In case other clients write un-related data under this path, maybe we don't
want to crash RM because of this.
bq.Is it really necessary to have "LOG.info("Storing info for " to be at the
INFO level? DEBUG maybe?
Change the store* and remove* methods to use debug
bq.the version info from getData() seems to be unused for future setData calls?
It seems like setData is only called from the unit tests?
You mean the stat info ? yes, it's of no use now. setData is only used in
unit tests.
bq. 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
fixed
> 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.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
