-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13754/#review25443
-----------------------------------------------------------
Below are a bunch of thoughts on the design. We should discuss them offline as
well. Pasting suggestions from Kishore below for further discussion:
- CLUSTER
- PARTICIPANT(*)
- CONFIG
- SPECTATOR(*)
- CONTROLLER(*)
- RESOURCE
- CONFIG
- IDEALSTATE
- EXTERNALVIEW
- CURRENTSTATE
State machine and constraints can be a separate dimension.
helix-core/src/main/java/org/apache/helix/api/Cluster.java
<https://reviews.apache.org/r/13754/#comment49845>
Specify what maps to what. In this case I guess it's name to object.
helix-core/src/main/java/org/apache/helix/api/ClusterReader.java
<https://reviews.apache.org/r/13754/#comment49844>
Maybe add some basic sanity checks as unit tests. I can help with that too.
helix-core/src/main/java/org/apache/helix/api/ExtView.java
<https://reviews.apache.org/r/13754/#comment49846>
A thought would be to make ExternalView a collection of CurrentStates. On
the other hand, maybe this is also OK because it matches the IdealState
interface.
helix-core/src/main/java/org/apache/helix/api/LeaderController.java
<https://reviews.apache.org/r/13754/#comment49843>
Please use the code format template.
helix-core/src/main/java/org/apache/helix/api/LiveParticipant.java
<https://reviews.apache.org/r/13754/#comment49847>
Is a separate object for live participants necessary? It could be a method
in participant called isLive and then cluster could implement
getLiveParticipants and filter internally
helix-core/src/main/java/org/apache/helix/api/Model.java
<https://reviews.apache.org/r/13754/#comment49842>
Is there a particular reason every class should derive from Model? It might
confuse users. Does it buy us any flexibility?
helix-core/src/main/java/org/apache/helix/api/Msg.java
<https://reviews.apache.org/r/13754/#comment49848>
Are messages necessary to expose in this API?
helix-core/src/main/java/org/apache/helix/api/Spectator.java
<https://reviews.apache.org/r/13754/#comment49849>
should this extend RunningInstance?
helix-core/src/main/java/org/apache/helix/api/StateModelDef.java
<https://reviews.apache.org/r/13754/#comment49850>
what about transitions?
- Kanak Biscuitwala
On Aug. 23, 2013, 12:02 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13754/
> -----------------------------------------------------------
>
> (Updated Aug. 23, 2013, 12:02 a.m.)
>
>
> Review request for helix, Kanak Biscuitwala, Kishore Gopalakrishna, and Shi
> Lu.
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> Initial change for refactor helix model. Roughly follow the design at:
> https://cwiki.apache.org/confluence/display/HELIX/API+Redesign
>
> An example is shown in ClusterReader where using a Cluster instance, we can
> retrieve any information needed for the rebalancer. The change is incomplete.
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/api/Cluster.java e69de29
> helix-core/src/main/java/org/apache/helix/api/ClusterReader.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Controller.java e69de29
> helix-core/src/main/java/org/apache/helix/api/CurState.java e69de29
> helix-core/src/main/java/org/apache/helix/api/ExtView.java e69de29
> helix-core/src/main/java/org/apache/helix/api/HelixVersion.java e69de29
> helix-core/src/main/java/org/apache/helix/api/LeaderController.java e69de29
> helix-core/src/main/java/org/apache/helix/api/LiveParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Model.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Msg.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Participant.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Partition.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Resource.java e69de29
> helix-core/src/main/java/org/apache/helix/api/RunningInstance.java e69de29
> helix-core/src/main/java/org/apache/helix/api/Spectator.java e69de29
> helix-core/src/main/java/org/apache/helix/api/StateModelDef.java e69de29
>
> Diff: https://reviews.apache.org/r/13754/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhen Zhang
>
>
