----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14728/#review27152 -----------------------------------------------------------
Looks good overall. This is probably going to require significant testing. Does it include all the fixes you made in the master branch? helix-core/src/main/java/org/apache/helix/HelixAutoController.java <https://reviews.apache.org/r/14728/#comment52837> probably should be under the api package somewhere helix-core/src/main/java/org/apache/helix/HelixAutoController.java <https://reviews.apache.org/r/14728/#comment52836> Maybe describe what an autonomous controller is and why it differs from a standard controller. helix-core/src/main/java/org/apache/helix/HelixConnection.java <https://reviews.apache.org/r/14728/#comment52839> would like this to extend HelixLockable so that you can get a HelixLock helix-core/src/main/java/org/apache/helix/HelixConnection.java <https://reviews.apache.org/r/14728/#comment52842> what about createSpectator? helix-core/src/main/java/org/apache/helix/manager/zk/HelixConnectionAdaptor.java <https://reviews.apache.org/r/14728/#comment52844> will any of the HelixConnection calls call the functions that throw UnsupportedOperationException? If so, they should be clearly marked helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixAutoController.java <https://reviews.apache.org/r/14728/#comment52847> javadoc helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java <https://reviews.apache.org/r/14728/#comment52849> I would have liked to have seen this use ZKHelixManager internally instead of duplicating functionality. We risk having to fix the same bug in 2 places. - Kanak Biscuitwala On Oct. 17, 2013, 2:38 p.m., Zhen Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14728/ > ----------------------------------------------------------- > > (Updated Oct. 17, 2013, 2:38 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/manager/zk/HelixConnectionAdaptor.java > e69de29 > > 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/participant/statemachine/HelixStateModelFactory.java > e69de29 > > helix-core/src/main/java/org/apache/helix/participant/statemachine/HelixStateModelFactoryAdaptor.java > e69de29 > helix-core/src/test/java/org/apache/helix/AppTest.java e69de29 > > Diff: https://reviews.apache.org/r/14728/diff/ > > > Testing > ------- > > AppTest > > > Thanks, > > Zhen Zhang > >
