You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by Kanak Biscuitwala <ka...@apache.org> on 2014/08/18 21:12:44 UTC

Review Request 24811: [HELIX-499] Controller should listen for all config changes

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24811/
-----------------------------------------------------------

Review request for helix, Zhen Zhang and Kishore Gopalakrishna.


Bugs: HELIX-499


Repository: helix-git


Description
-------

commit 173f207dc452a3f00d7e6c3ff78176579be06d8d
Author: Kanak Biscuitwala <ka...@apache.org>
Date:   Mon Aug 18 12:08:44 2014 -0700

    [HELIX-499] Controller should listen for all config changes

:100644 100644 421ff60... 2b59b27... M	helix-core/src/main/java/org/apache/helix/api/Cluster.java
:100644 100644 ddf809a... eac2bf8... M	helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java
:100644 100644 f1c2583... d36b6f5... M	helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
:100644 100644 ca540c5... b652f35... M	helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java
:100644 100644 877baf2... 53ddd19... M	helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
:100644 100644 623b874... e2def60... M	helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java
:100644 100644 ee420b9... b953e0b... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java
:100644 100644 0a9dc94... 38332c5... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java
:100644 100644 0698945... 295b69c... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java
:100644 100644 8f861cd... af89944... M	helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java
:100644 100644 17b610d... cca7d76... M	helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
:100644 100644 9e8fd85... 190f739... M	helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
:100644 000000 1393231... 0000000... D	helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java


Diffs
-----

  helix-core/src/main/java/org/apache/helix/api/Cluster.java 421ff60 
  helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java ddf809a 
  helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java f1c2583 
  helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java ca540c5 
  helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java 877baf2 
  helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java 623b874 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java ee420b9 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java 0a9dc94 
  helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java 0698945 
  helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java 8f861cd 
  helix-core/src/main/java/org/apache/helix/task/TaskUtil.java 17b610d 
  helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java 9e8fd85 
  helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java 1393231 

Diff: https://reviews.apache.org/r/24811/diff/


Testing
-------

mvn test


Thanks,

Kanak Biscuitwala


Re: Review Request 24811: [HELIX-499] Controller should listen for all config changes

Posted by Kanak Biscuitwala <ka...@apache.org>.

> On Aug. 18, 2014, 12:20 p.m., Kishore Gopalakrishna wrote:
> > why did we remove _cache and testzkcallbacklead

_cache was added, not removed.

The test was removed because it was code duplication with another test of the same name, different package.


> On Aug. 18, 2014, 12:20 p.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java, line 221
> > <https://reviews.apache.org/r/24811/diff/1/?file=662677#file662677line221>
> >
> >     Can we create a enum/constant for event names. probably a separate jira

Created a separate jira.


- Kanak


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24811/#review50924
-----------------------------------------------------------


On Aug. 18, 2014, 12:12 p.m., Kanak Biscuitwala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24811/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2014, 12:12 p.m.)
> 
> 
> Review request for helix, Zhen Zhang and Kishore Gopalakrishna.
> 
> 
> Bugs: HELIX-499
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> commit 173f207dc452a3f00d7e6c3ff78176579be06d8d
> Author: Kanak Biscuitwala <ka...@apache.org>
> Date:   Mon Aug 18 12:08:44 2014 -0700
> 
>     [HELIX-499] Controller should listen for all config changes
> 
> :100644 100644 421ff60... 2b59b27... M	helix-core/src/main/java/org/apache/helix/api/Cluster.java
> :100644 100644 ddf809a... eac2bf8... M	helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java
> :100644 100644 f1c2583... d36b6f5... M	helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
> :100644 100644 ca540c5... b652f35... M	helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java
> :100644 100644 877baf2... 53ddd19... M	helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
> :100644 100644 623b874... e2def60... M	helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java
> :100644 100644 ee420b9... b953e0b... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java
> :100644 100644 0a9dc94... 38332c5... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java
> :100644 100644 0698945... 295b69c... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java
> :100644 100644 8f861cd... af89944... M	helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java
> :100644 100644 17b610d... cca7d76... M	helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
> :100644 100644 9e8fd85... 190f739... M	helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
> :100644 000000 1393231... 0000000... D	helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/api/Cluster.java 421ff60 
>   helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java ddf809a 
>   helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java f1c2583 
>   helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java ca540c5 
>   helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java 877baf2 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java 623b874 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java ee420b9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java 0a9dc94 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java 0698945 
>   helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java 8f861cd 
>   helix-core/src/main/java/org/apache/helix/task/TaskUtil.java 17b610d 
>   helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java 9e8fd85 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java 1393231 
> 
> Diff: https://reviews.apache.org/r/24811/diff/
> 
> 
> Testing
> -------
> 
> mvn test
> 
> 
> Thanks,
> 
> Kanak Biscuitwala
> 
>


Re: Review Request 24811: [HELIX-499] Controller should listen for all config changes

Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24811/#review50924
-----------------------------------------------------------

Ship it!


why did we remove _cache and testzkcallbacklead


helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
<https://reviews.apache.org/r/24811/#comment88808>

    Can we create a enum/constant for event names. probably a separate jira



helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
<https://reviews.apache.org/r/24811/#comment88809>

    Love this test :-)


wh

- Kishore Gopalakrishna


On Aug. 18, 2014, 7:12 p.m., Kanak Biscuitwala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24811/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2014, 7:12 p.m.)
> 
> 
> Review request for helix, Zhen Zhang and Kishore Gopalakrishna.
> 
> 
> Bugs: HELIX-499
> 
> 
> Repository: helix-git
> 
> 
> Description
> -------
> 
> commit 173f207dc452a3f00d7e6c3ff78176579be06d8d
> Author: Kanak Biscuitwala <ka...@apache.org>
> Date:   Mon Aug 18 12:08:44 2014 -0700
> 
>     [HELIX-499] Controller should listen for all config changes
> 
> :100644 100644 421ff60... 2b59b27... M	helix-core/src/main/java/org/apache/helix/api/Cluster.java
> :100644 100644 ddf809a... eac2bf8... M	helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java
> :100644 100644 f1c2583... d36b6f5... M	helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
> :100644 100644 ca540c5... b652f35... M	helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java
> :100644 100644 877baf2... 53ddd19... M	helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
> :100644 100644 623b874... e2def60... M	helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java
> :100644 100644 ee420b9... b953e0b... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java
> :100644 100644 0a9dc94... 38332c5... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java
> :100644 100644 0698945... 295b69c... M	helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java
> :100644 100644 8f861cd... af89944... M	helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java
> :100644 100644 17b610d... cca7d76... M	helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
> :100644 100644 9e8fd85... 190f739... M	helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java
> :100644 000000 1393231... 0000000... D	helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java
> 
> 
> Diffs
> -----
> 
>   helix-core/src/main/java/org/apache/helix/api/Cluster.java 421ff60 
>   helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java ddf809a 
>   helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java f1c2583 
>   helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java ca540c5 
>   helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java 877baf2 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ControllerManagerHelper.java 623b874 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkCallbackHandler.java ee420b9 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java 0a9dc94 
>   helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java 0698945 
>   helix-core/src/main/java/org/apache/helix/task/TaskRebalancer.java 8f861cd 
>   helix-core/src/main/java/org/apache/helix/task/TaskUtil.java 17b610d 
>   helix-core/src/test/java/org/apache/helix/integration/TestZkCallbackHandlerLeak.java 9e8fd85 
>   helix-core/src/test/java/org/apache/helix/integration/manager/TestZkCallbackHandlerLeak.java 1393231 
> 
> Diff: https://reviews.apache.org/r/24811/diff/
> 
> 
> Testing
> -------
> 
> mvn test
> 
> 
> Thanks,
> 
> Kanak Biscuitwala
> 
>