[GitHub] [helix] narendly commented on a change in pull request #534: Implementation of create/get/set methods with custom serializer support

[GitBox <gi...@apache.org>]

narendly commented on a change in pull request #534: Implementation of create/get/set methods with custom serializer support
URL: https://github.com/apache/helix/pull/534#discussion_r343841618
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java
 ##########
 @@ -92,19 +98,42 @@ public ZkBaseDataAccessor(HelixZkClient zkClient) {
     _zkClient = zkClient;
   }
 
+  private synchronized HelixZkClient getNonZNRecordClient() {
+    if (_nonZNRecordClient == null) {
 
 Review comment:
   I think the use of synchronization primitives could be changed to allow for more parallelism?
   
   Instead of synchronizing on the whole method, could we do:
   
   ```
   private HelixZkClient getNonZNRecordZkClient() {
     if (_nonZnRecordClient == null) {
       synchronized (`_nonZnRecordZkClient`) {
         // create...
     else {
       return `_nonZnRecordZkClient`;
   ```
   
   Synchronizing the whole method might significantly reduce parallelism. We only need to synchronize when we need to modify the underlying object, which is `_nonZnRecordZkClient`;

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org