----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17670/#review34893 -----------------------------------------------------------
helix-core/src/main/java/org/apache/helix/api/accessor/AtomicParticipantAccessor.java <https://reviews.apache.org/r/17670/#comment65269> Why track the clusterId here if we are passing it in super? Please remove. helix-core/src/main/java/org/apache/helix/api/accessor/ParticipantAccessor.java <https://reviews.apache.org/r/17670/#comment65271> We invoke toString on the clusterId in methods but we dont check for nulls. helix-core/src/main/java/org/apache/helix/api/accessor/ParticipantAccessor.java <https://reviews.apache.org/r/17670/#comment65273> Is there a reason this is package scoped and not protected? helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java <https://reviews.apache.org/r/17670/#comment65272> check if logger.isInfoEnabled helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java <https://reviews.apache.org/r/17670/#comment65274> Please check if logger.isInfoEnabled helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java <https://reviews.apache.org/r/17670/#comment65276> If isConnected is checking for _zkClient != null in a lock, should this not be done within a lock? helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java <https://reviews.apache.org/r/17670/#comment65275> I understand this was not changed in this code change but can we add a check if log.isInfoEnabled? - Sandeep Nayak On Feb. 3, 2014, 7:25 p.m., Kanak Biscuitwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17670/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2014, 7:25 p.m.) > > > Review request for helix, Zhen Zhang and Kishore Gopalakrishna. > > > Bugs: HELIX-360 > > > Repository: helix-git > > > Description > ------- > > commit 0d148843f4c55e255ac51160d9dfb2b3047eb3dd > Author: Kanak Biscuitwala <[email protected]> > Date: Mon Feb 3 11:21:55 2014 -0800 > > [HELIX-360] Remove code duplication for list of required paths > > :100644 100644 1c734e3... 90f58ea... M > helix-core/src/main/java/org/apache/helix/api/accessor/AtomicParticipantAccessor.java > :100644 100644 cda83d8... 48457b2... M > helix-core/src/main/java/org/apache/helix/api/accessor/AtomicResourceAccessor.java > :100644 100644 36c7aaa... abb3e49... M > helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java > :100644 100644 c3deafe... 3a34ca2... M > helix-core/src/main/java/org/apache/helix/api/accessor/ParticipantAccessor.java > :100644 100644 80c5b16... 73d43b0... M > helix-core/src/main/java/org/apache/helix/api/accessor/ResourceAccessor.java > :100644 100644 45fc355... b3252a8... M > helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java > :100644 100644 9018416... 30ee16c... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java > :100644 100644 1bdc54c... d59fad7... M > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java > :100644 100644 ba8958d... ea28c76... M > helix-core/src/main/java/org/apache/helix/tools/NewClusterSetup.java > :100644 100644 4eebbc6... ee51834... M > helix-core/src/test/java/org/apache/helix/api/accessor/TestAccessorRecreate.java > > > Diffs > ----- > > > helix-core/src/main/java/org/apache/helix/api/accessor/AtomicParticipantAccessor.java > 1c734e3 > > helix-core/src/main/java/org/apache/helix/api/accessor/AtomicResourceAccessor.java > cda83d8 > helix-core/src/main/java/org/apache/helix/api/accessor/ClusterAccessor.java > 36c7aaa > > helix-core/src/main/java/org/apache/helix/api/accessor/ParticipantAccessor.java > c3deafe > > helix-core/src/main/java/org/apache/helix/api/accessor/ResourceAccessor.java > 80c5b16 > > helix-core/src/main/java/org/apache/helix/controller/stages/PersistAssignmentStage.java > 45fc355 > helix-core/src/main/java/org/apache/helix/manager/zk/ZKUtil.java 9018416 > helix-core/src/main/java/org/apache/helix/manager/zk/ZkHelixConnection.java > 1bdc54c > helix-core/src/main/java/org/apache/helix/tools/NewClusterSetup.java > ba8958d > > helix-core/src/test/java/org/apache/helix/api/accessor/TestAccessorRecreate.java > 4eebbc6 > > Diff: https://reviews.apache.org/r/17670/diff/ > > > Testing > ------- > > > Thanks, > > Kanak Biscuitwala > >
