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 2013/11/06 02:53:14 UTC
Re: Review Request 14728: [HELIX-259] HelixConnection
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14728/
-----------------------------------------------------------
(Updated Nov. 6, 2013, 1:53 a.m.)
Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
Changes
-------
updated diff
Repository: helix-git
Description
-------
replace HelixManager with HelixConnection so we can reuse connections. major interfaces are:
HelixConnection
HelixConnectionStateListener
HelixService
HelixRole
HelixParticipant
HelixController
HelixAutoController
AppTest shows an example of using new APIs to setup and start a cluster
Diffs (updated)
-----
helix-core/src/main/java/org/apache/helix/HelixAutoController.java e69de29
helix-core/src/main/java/org/apache/helix/HelixConnection.java e69de29
helix-core/src/main/java/org/apache/helix/HelixConnectionStateListener.java e69de29
helix-core/src/main/java/org/apache/helix/HelixController.java e69de29
helix-core/src/main/java/org/apache/helix/HelixParticipant.java e69de29
helix-core/src/main/java/org/apache/helix/HelixRole.java e69de29
helix-core/src/main/java/org/apache/helix/HelixService.java e69de29
helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java 85b8432
helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java e69de29
helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java 0b112cd
helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java e69de29
helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java e69de29
helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java e69de29
helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java e69de29
helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java e69de29
helix-core/src/main/java/org/apache/helix/monitoring/StatusDumpTask.java e69de29
helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 4e4fdf6
helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java d11b3cc
helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactory.java e69de29
helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactoryAdaptor.java e69de29
helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java 4d8e598
helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java e69de29
Diff: https://reviews.apache.org/r/14728/diff/
Testing
-------
AppTest
Thanks,
Zhen Zhang
Re: Review Request 14728: [HELIX-259] HelixConnection
Posted by Kanak Biscuitwala <ka...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14728/#review28250
-----------------------------------------------------------
Ship it!
Ship It!
- Kanak Biscuitwala
On Nov. 5, 2013, 5:53 p.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14728/
> -----------------------------------------------------------
>
> (Updated Nov. 5, 2013, 5:53 p.m.)
>
>
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> replace HelixManager with HelixConnection so we can reuse connections. major interfaces are:
>
> HelixConnection
> HelixConnectionStateListener
>
> HelixService
> HelixRole
> HelixParticipant
> HelixController
> HelixAutoController
>
> AppTest shows an example of using new APIs to setup and start a cluster
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/HelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnectionStateListener.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixRole.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixService.java e69de29
> helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java 85b8432
> helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java 0b112cd
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/monitoring/StatusDumpTask.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 4e4fdf6
> helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java d11b3cc
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactory.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactoryAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java 4d8e598
> helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java e69de29
>
> Diff: https://reviews.apache.org/r/14728/diff/
>
>
> Testing
> -------
>
> AppTest
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 14728: [HELIX-259] HelixConnection
Posted by Zhen Zhang <ne...@gmail.com>.
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixAutoController.java, line 9
> > <https://reviews.apache.org/r/14728/diff/2/?file=378706#file378706line9>
> >
> > Is there a different type of controller, what does autocontroller mean ?
auto controller is the original controller participant, which means autonomous controller
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixConnection.java, line 17
> > <https://reviews.apache.org/r/14728/diff/2/?file=378707#file378707line17>
> >
> > Can we have an enum defining the state of HelixConnection ?, it can be connected, connecting, disconnected, expired etc. Allows users to use this for additional logic
that makes sense. will add it
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixConnection.java, line 88
> > <https://reviews.apache.org/r/14728/diff/2/?file=378707#file378707line88>
> >
> > createClusterManagment -- Typo
> > calling HelixAdmin createClusterManagementTool lets remove that or deprecate it.Can we not simply call it HelixAdmin ?
> >
will think of that
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixConnection.java, line 102
> > <https://reviews.apache.org/r/14728/diff/2/?file=378707#file378707line102>
> >
> > whats the difference between HelixDataAccessor and ParticipantAccessor, ResourceAccessor, ClusterAccessor.
> >
> > Will it be better to get the specificAccessor from HelixDataAccessor ?
> >
> >
HelixDataAccessor is the original data accessor that can be used to get/set any helix property
ParticipantAccessor, ResourceAccessor, and ClusterAccessor are logical accessors. They are more used for logical operations. For example, disable a participant will be done via ParticipantAccessor.disableParticipant(..)
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixConnection.java, line 110
> > <https://reviews.apache.org/r/14728/diff/2/?file=378707#file378707line110>
> >
> > can we get specificAccessor from HelixDataAccessor
sure
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixConnection.java, line 143
> > <https://reviews.apache.org/r/14728/diff/2/?file=378707#file378707line143>
> >
> > What is HelixRole ? Should we have these addListener methods in the HelixRole ?
HelixRole serves as the base class for HelixParticipant, HelixController, and HelixAutoController.
We might move the addListener methods to HelixRole. It depends on conceptually whether the listeners belong to a connection or a role (i.e. participant/controller). HelixConnection is the listener that receives callbacks from zookeeper, so I put the listeners here, but HelixConnection can also pass the callbacks to HelixRole.
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixRole.java, line 38
> > <https://reviews.apache.org/r/14728/diff/2/?file=378711#file378711line38>
> >
> > why messagingService here ?
All HelixRole (participant/controller/auto-controller) provides messaging service, so we can register message handlers.
> On Nov. 6, 2013, 3:39 a.m., Kishore Gopalakrishna wrote:
> > helix-core/src/main/java/org/apache/helix/HelixService.java, line 10
> > <https://reviews.apache.org/r/14728/diff/2/?file=378712#file378712line10>
> >
> > what does Async mean, how to wait until its started
For async it means we just create the live-instance. For sync, we can probably wait until the external view becomes the best possible state.
- Zhen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14728/#review28253
-----------------------------------------------------------
On Nov. 6, 2013, 1:53 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14728/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:53 a.m.)
>
>
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> replace HelixManager with HelixConnection so we can reuse connections. major interfaces are:
>
> HelixConnection
> HelixConnectionStateListener
>
> HelixService
> HelixRole
> HelixParticipant
> HelixController
> HelixAutoController
>
> AppTest shows an example of using new APIs to setup and start a cluster
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/HelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnectionStateListener.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixRole.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixService.java e69de29
> helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java 85b8432
> helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java 0b112cd
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/monitoring/StatusDumpTask.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 4e4fdf6
> helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java d11b3cc
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactory.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactoryAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java 4d8e598
> helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java e69de29
>
> Diff: https://reviews.apache.org/r/14728/diff/
>
>
> Testing
> -------
>
> AppTest
>
>
> Thanks,
>
> Zhen Zhang
>
>
Re: Review Request 14728: [HELIX-259] HelixConnection
Posted by Kishore Gopalakrishna <ki...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14728/#review28253
-----------------------------------------------------------
Great progress.Reviewed only the interfaces. Will review the remaining after getting ur feedback on the review comments
helix-core/src/main/java/org/apache/helix/HelixAutoController.java
<https://reviews.apache.org/r/14728/#comment54967>
Is there a different type of controller, what does autocontroller mean ?
helix-core/src/main/java/org/apache/helix/HelixConnection.java
<https://reviews.apache.org/r/14728/#comment54968>
Can we have an enum defining the state of HelixConnection ?, it can be connected, connecting, disconnected, expired etc. Allows users to use this for additional logic
helix-core/src/main/java/org/apache/helix/HelixConnection.java
<https://reviews.apache.org/r/14728/#comment54969>
createClusterManagment -- Typo
calling HelixAdmin createClusterManagementTool lets remove that or deprecate it.Can we not simply call it HelixAdmin ?
helix-core/src/main/java/org/apache/helix/HelixConnection.java
<https://reviews.apache.org/r/14728/#comment54970>
whats the difference between HelixDataAccessor and ParticipantAccessor, ResourceAccessor, ClusterAccessor.
Will it be better to get the specificAccessor from HelixDataAccessor ?
helix-core/src/main/java/org/apache/helix/HelixConnection.java
<https://reviews.apache.org/r/14728/#comment54971>
can we get specificAccessor from HelixDataAccessor
helix-core/src/main/java/org/apache/helix/HelixConnection.java
<https://reviews.apache.org/r/14728/#comment54972>
What is HelixRole ? Should we have these addListener methods in the HelixRole ?
helix-core/src/main/java/org/apache/helix/HelixRole.java
<https://reviews.apache.org/r/14728/#comment54973>
why messagingService here ?
helix-core/src/main/java/org/apache/helix/HelixService.java
<https://reviews.apache.org/r/14728/#comment54974>
what does Async mean, how to wait until its started
- Kishore Gopalakrishna
On Nov. 6, 2013, 1:53 a.m., Zhen Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14728/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2013, 1:53 a.m.)
>
>
> Review request for helix, Kanak Biscuitwala and Kishore Gopalakrishna.
>
>
> Repository: helix-git
>
>
> Description
> -------
>
> replace HelixManager with HelixConnection so we can reuse connections. major interfaces are:
>
> HelixConnection
> HelixConnectionStateListener
>
> HelixService
> HelixRole
> HelixParticipant
> HelixController
> HelixAutoController
>
> AppTest shows an example of using new APIs to setup and start a cluster
>
>
> Diffs
> -----
>
> helix-core/src/main/java/org/apache/helix/HelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixConnectionStateListener.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixRole.java e69de29
> helix-core/src/main/java/org/apache/helix/HelixService.java e69de29
> helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java 85b8432
> helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java 0b112cd
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixController.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixLeaderElection.java e69de29
> helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixParticipant.java e69de29
> helix-core/src/main/java/org/apache/helix/monitoring/StatusDumpTask.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/HelixStateMachineEngine.java 4e4fdf6
> helix-core/src/main/java/org/apache/helix/participant/StateMachineEngine.java d11b3cc
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactory.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactoryAdaptor.java e69de29
> helix-core/src/main/java/org/apache/helix/participant/statemachine/StateModelFactory.java 4d8e598
> helix-core/src/test/java/org/apache/helix/integration/TestHelixConnection.java e69de29
>
> Diff: https://reviews.apache.org/r/14728/diff/
>
>
> Testing
> -------
>
> AppTest
>
>
> Thanks,
>
> Zhen Zhang
>
>