You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by Zhen Zhang <ne...@gmail.com> on 2015/02/25 02:39:24 UTC
Review Request 31397: [HELIX-570] Add default state model definitions
if not already exists when controller starts
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/
-----------------------------------------------------------
Review request for helix, Kishore Gopalakrishna and Shi Lu.
Bugs: 570
Repository: helix-git
Description
-------
When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
Diffs
-----
helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
Diff: https://reviews.apache.org/r/31397/diff/
Testing
-------
mvn test
Thanks,
Zhen Zhang
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Kishore Gopalakrishna <ki...@apache.org>.
> On Feb. 25, 2015, 11:13 p.m., Zhen Zhang wrote:
> > helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java, line 153
> > <https://reviews.apache.org/r/31397/diff/1/?file=874721#file874721line153>
> >
> > how about BuildinStateModelDefinitions
BuiltInSMD works.
- Kishore
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/#review74136
-----------------------------------------------------------
On Feb. 25, 2015, 1:39 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31397/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2015, 1:39 a.m.)
>
>
> Review request for helix, Kishore Gopalakrishna and Shi Lu.
>
>
> Bugs: 570
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
> helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
> helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
> helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
> helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/31397/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Zhen Zhang <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/#review74136
-----------------------------------------------------------
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120665>
will reorg the class
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120664>
will fix it.
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120663>
CONTROLLER and CONTROLLER_PARTICIPANT have the same code. Just clean it up.
helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
<https://reviews.apache.org/r/31397/#comment120661>
how about BuildinStateModelDefinitions
helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java
<https://reviews.apache.org/r/31397/#comment120666>
will fix this.
- Zhen Zhang
On Feb. 25, 2015, 1:39 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31397/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2015, 1:39 a.m.)
>
>
> Review request for helix, Kishore Gopalakrishna and Shi Lu.
>
>
> Bugs: 570
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
> helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
> helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
> helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
> helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/31397/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Shi Lu <lu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/#review73959
-----------------------------------------------------------
Ship it!
Ship It!
- Shi Lu
On Feb. 25, 2015, 1:39 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31397/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2015, 1:39 a.m.)
>
>
> Review request for helix, Kishore Gopalakrishna and Shi Lu.
>
>
> Bugs: 570
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
> helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
> helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
> helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
> helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/31397/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Zhen Zhang <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/
-----------------------------------------------------------
(Updated March 17, 2015, 11:49 p.m.)
Review request for helix, Kishore Gopalakrishna and Shi Lu.
Bugs: 570
Repository: helix-git
Description
-------
When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
Diffs (updated)
-----
helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
helix-core/src/main/java/org/apache/helix/model/BuildInSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/BuiltInStateModelDefinitions.java e69de29
helix-core/src/main/java/org/apache/helix/model/LeaderStandbySMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/MasterSlaveSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/OnlineOfflineSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/ScheduledTaskSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/StateModelDefinition.java e99e173
helix-core/src/main/java/org/apache/helix/model/StorageSchemataSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/TaskSMD.java e69de29
helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
helix-core/src/test/java/org/apache/helix/integration/IntegrationTest.java bb862f3
helix-core/src/test/java/org/apache/helix/manager/zk/TestAddBuiltInStateModelDef.java e69de29
helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
Diff: https://reviews.apache.org/r/31397/diff/
Testing
-------
mvn test
Thanks,
Zhen Zhang
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/#review74862
-----------------------------------------------------------
Ship it!
helix-core/src/main/java/org/apache/helix/model/BuildInSMD.java
<https://reviews.apache.org/r/31397/#comment121674>
rename to BuiltInStateModelDefinitions or StateModelDefinitionsProvided, we also have statemodeldefinitionBuilder, i think its will be nice to provide methods on the builder for every statemodel
- Kishore Gopalakrishna
On Feb. 26, 2015, 2:23 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31397/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2015, 2:23 a.m.)
>
>
> Review request for helix, Kishore Gopalakrishna and Shi Lu.
>
>
> Bugs: 570
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
> helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
> helix-core/src/main/java/org/apache/helix/model/BuildInSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/LeaderStandbySMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/MasterSlaveSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/OnlineOfflineSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/ScheduledTaskSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/StorageSchemataSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/model/TaskSMD.java e69de29
> helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
> helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
> helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/31397/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Zhen Zhang <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/
-----------------------------------------------------------
(Updated Feb. 26, 2015, 2:23 a.m.)
Review request for helix, Kishore Gopalakrishna and Shi Lu.
Changes
-------
apply comments
Bugs: 570
Repository: helix-git
Description
-------
When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
Diffs (updated)
-----
helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
helix-core/src/main/java/org/apache/helix/model/BuildInSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/LeaderStandbySMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/MasterSlaveSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/OnlineOfflineSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/ScheduledTaskSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/StorageSchemataSMD.java e69de29
helix-core/src/main/java/org/apache/helix/model/TaskSMD.java e69de29
helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
Diff: https://reviews.apache.org/r/31397/diff/
Testing
-------
mvn test
Thanks,
Zhen Zhang
Re: Review Request 31397: [HELIX-570] Add default state model
definitions if not already exists when controller starts
Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31397/#review73966
-----------------------------------------------------------
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120370>
can we avoid bringing code from tools into core? tools package has lot of unwanted code and we should move it out into helix-tools module. when we do that helix-tools will depend on helix-core
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120374>
we should call it addDefaultStateModelDefinitions()
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
<https://reviews.apache.org/r/31397/#comment120371>
why is this removed as part of this rb?
helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
<https://reviews.apache.org/r/31397/#comment120372>
we need to chose a better name, we DefaultStateModelDefinition seems like a one that we would use if no state model is specified.
helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java
<https://reviews.apache.org/r/31397/#comment120373>
I like that we have one place to view all the statemodels, we should get rid of state model config generator class and have one class for each statemodeldefinition
- Kishore Gopalakrishna
On Feb. 25, 2015, 1:39 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31397/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2015, 1:39 a.m.)
>
>
> Review request for helix, Kishore Gopalakrishna and Shi Lu.
>
>
> Bugs: 570
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> When controller starts, we can add missing default state model definitions to the cluster. This is useful for example, a legacy cluster is created without some new default state model definitions. Instead of building a tool to populate the new default state model definitions to existing clusters, controller can check this and add it if necessary.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/controller/HelixControllerMain.java b6c16b5
> helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java 3328279
> helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java 40bc398
> helix-core/src/main/java/org/apache/helix/tools/StateModelConfigGenerator.java b8b3aeb
> helix-core/src/test/java/org/apache/helix/manager/zk/TestAddDefaultStateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/31397/diff/
>
>
> Testing
> -------
>
> mvn test
>
>
> Thanks,
>
> Zhen Zhang
>
>