[
https://issues.apache.org/jira/browse/YARN-6840?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16165563#comment-16165563
]
Jonathan Hung edited comment on YARN-6840 at 9/14/17 1:06 AM:
--------------------------------------------------------------
Thanks [~leftnoteasy] for the review. Attached 004 addressing most of these
comments. Also added a couple of sleeps in TestZKConfigurationStore due to a
race condition to fix some test failures.
bq. Move following code: To refreshAll, and parameter isActiveTransition and
private void refreshQueues can be removed.
Done
bq. Is it a better idea to move public void refreshQueues() logic to
scheduler#reinitialize(...). It's not a good idea to invoke
AdminService#refreshQueues from MutableCSConfigurationProvider.
Not sure about this, then we are doing the reservation system reinitialization
inside scheduler, so every time scheduler#reinitialize is called, the
reservation system is also initialized, not sure if this is the desired
behavior. Also we would need to duplicate the reservation system
reinitialization for all schedulers, or make ResourceScheduler an abstract
class and add it there.
Unless you meant duplicating the reservation system reinitialization logic
inside MutableCSConfigurationProvider#mutateConfiguration? I think this makes
more sense, but then we have duplicate code between this and AdminService.
bq. In MutableCSConfigurationProvider: It's better to remove:...
Done
bq. In MutableConfScheduler: Similar to above, it's better to remove:...
Done
bq. refreshConfiguration -> reloadConfigurationFromStore
Done
bq. createAndStartZKManager can be merged to rm#getZKManager()
Renamed to getAndStartZKManager
bq. Getter API of ResourceManager should be exposed by RMContext. We should
never use ref to ResourceManager directly.
Done for ZKConfigurationStore, I left the references in ZKRMStateStore since
this class has other direct references to rm object. We can handle this in a
separate ticket if you'd like.
bq. setResourceManager can be removed and you can pass RMContext ref to
initialize.
Done
bq. What happens if Configuration schedConf passed to a already initialized
store?
For leveldb and zk, it will ignore it and use the scheduler configuration
persisted in the store.
bq. Could you add Javadocs to following methods:
Done
was (Author: jhung):
Thanks [~leftnoteasy] for the review. Attached 004 addressing most of these
comments:
bq. Move following code: To refreshAll, and parameter isActiveTransition and
private void refreshQueues can be removed.
Done
bq. Is it a better idea to move public void refreshQueues() logic to
scheduler#reinitialize(...). It's not a good idea to invoke
AdminService#refreshQueues from MutableCSConfigurationProvider.
Not sure about this, then we are doing the reservation system reinitialization
inside scheduler, so every time scheduler#reinitialize is called, the
reservation system is also initialized, not sure if this is the desired
behavior. Also we would need to duplicate the reservation system
reinitialization for all schedulers, or make ResourceScheduler an abstract
class and add it there.
Unless you meant duplicating the reservation system reinitialization logic
inside MutableCSConfigurationProvider#mutateConfiguration? I think this makes
more sense, but then we have duplicate code between this and AdminService.
bq. In MutableCSConfigurationProvider: It's better to remove:...
Done
bq. In MutableConfScheduler: Similar to above, it's better to remove:...
Done
bq. refreshConfiguration -> reloadConfigurationFromStore
Done
bq. createAndStartZKManager can be merged to rm#getZKManager()
Renamed to getAndStartZKManager
bq. Getter API of ResourceManager should be exposed by RMContext. We should
never use ref to ResourceManager directly.
Done for ZKConfigurationStore, I left the references in ZKRMStateStore since
this class has other direct references to rm object. We can handle this in a
separate ticket if you'd like.
bq. setResourceManager can be removed and you can pass RMContext ref to
initialize.
Done
bq. What happens if Configuration schedConf passed to a already initialized
store?
For leveldb and zk, it will ignore it and use the scheduler configuration
persisted in the store.
bq. Could you add Javadocs to following methods:
Done
> Implement zookeeper based store for scheduler configuration updates
> -------------------------------------------------------------------
>
> Key: YARN-6840
> URL: https://issues.apache.org/jira/browse/YARN-6840
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Wangda Tan
> Assignee: Jonathan Hung
> Attachments: YARN-6840-YARN-5734.001.patch,
> YARN-6840-YARN-5734.002.patch, YARN-6840-YARN-5734.003.patch,
> YARN-6840-YARN-5734.004.patch
>
>
> Right now there is only in-memory and leveldb based configuration store
> supported. Need one which supports RM HA.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
