From notifications-return-500-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Wed Jun 26 15:45:18 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 844FC18064D for ; Wed, 26 Jun 2019 17:45:18 +0200 (CEST) Received: (qmail 95689 invoked by uid 500); 26 Jun 2019 15:45:18 -0000 Mailing-List: contact notifications-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list notifications@zookeeper.apache.org Received: (qmail 95676 invoked by uid 99); 26 Jun 2019 15:45:17 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 26 Jun 2019 15:45:17 +0000 From: GitBox To: notifications@zookeeper.apache.org Subject: [GitHub] [zookeeper] jhalterman commented on a change in pull request #1004: ZOOKEEPER-3445 ReferenceCountedACLCache.aclIndex should be volatile Message-ID: <156156391723.6400.13024352960362103510.gitbox@gitbox.apache.org> Date: Wed, 26 Jun 2019 15:45:17 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit jhalterman commented on a change in pull request #1004: ZOOKEEPER-3445 ReferenceCountedACLCache.aclIndex should be volatile URL: https://github.com/apache/zookeeper/pull/1004#discussion_r297737922 ########## File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ReferenceCountedACLCache.java ########## @@ -51,7 +51,7 @@ /** * these are the number of acls that we have in the datatree */ - long aclIndex = 0; + volatile long aclIndex = 0; Review comment: From what I can tell, the `FileTxnSnapLog#restore` call that happens on startup occurs in the main thread. Later, `Learner#syncWithLeader` occurs in the `QuorumPeer`'s thread. There appear to be other possible threads that touch `ReferenceCountedACLCache` as well. Ex: ```FollowerRequestProcessor.run->FileRequestProcessor.processRequest->ZookeeperServer.processTxn->ZkDatabase.processTxn->DataTree.processTxn->DataTree.createNode->ReferenceCountedACLCache.convertActls-> ReferenceCountedACLCache.incrementIndex``` and ```WorkerService.run->CommitProcessor.doWork->FileRequestProcessor.processRequest->ZookeeperServer.processTxn->ZkDatabase.processTxn->DataTree.processTxn->DataTree.createNode->ReferenceCountedACLCache.convertActls-> ReferenceCountedACLCache.incrementIndex``` But overall I'd ask, if `aclIndex` doesn't need to be threadsafe, then why is the rest of `ReferenceCountedACLCache` written to be threadsafe? ---------------------------------------------------------------- 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