Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Thu, 14 Sep 2017 01:07:00 +0000 (UTC)
From: "Jonathan Hung (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.13088005.1500402455000.107820.1505351220432@Atlassian.JIRA>
In-Reply-To: <JIRA.13088005.1500402455000@Atlassian.JIRA>
References: <JIRA.13088005.1500402455000@Atlassian.JIRA> <JIRA.13088005.1500402455185@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (YARN-6840) Implement zookeeper based store
 for scheduler configuration updates
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Thu, 14 Sep 2017 01:07:07 -0000


    [ 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-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org