[ https://issues.apache.org/jira/browse/CURATOR-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15122251#comment-15122251 ]
ASF GitHub Bot commented on CURATOR-294: ---------------------------------------- GitHub user dragonsinth opened a pull request: https://github.com/apache/curator/pull/127 CURATOR-294 @Randgalt You can merge this pull request into a Git repository by running: $ git pull https://github.com/dragonsinth/curator CURATOR-294 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/127.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #127 ---- commit cc27e3434de7dae58f9fb07a013137d0da3dc378 Author: Scott Blum <dragonsi...@apache.org> Date: 2016-01-28T17:38:57Z CURATOR-294: Make ChildData immutable; PathChildrenCache uses a mutable subclass. commit 81067f5b35937e4415b1d11e8d06b99a21f67926 Author: Scott Blum <dragonsi...@apache.org> Date: 2016-01-28T17:58:14Z CURATOR-294: Optimize TreeCache, fix possible concurrency issue ---- > Optimize TreeCache, fix possible concurrency issue > -------------------------------------------------- > > Key: CURATOR-294 > URL: https://issues.apache.org/jira/browse/CURATOR-294 > Project: Apache Curator > Issue Type: Improvement > Components: Recipes > Reporter: Nick Hill > Assignee: Scott Blum > Fix For: 3.0.1, 2.9.2 > > > Hi, I have been looking at the TreeCache impl and have some questions. > It doesn't look right to me that there's separate atomic refs for a node's > data and stat. It seems the stat in a ChildData object obtained from > getCurrentData() might not correspond to the data that it's with. This could > be problematic when doing conditional state changes given assumptions about > that data. > An obvious and simple solution to this would be to have a single > AtomicReference<ChildData> field instead, which would have the additional > significant benefit of eliminating ChildData obj creation on every cache > access. PathChildrenCache works this way, but my understanding was that > TreeCache is intended to be a (more flexible) replacement. > Furthermore I'd propose that the data field of ChildData be just a final > byte[] instead of an AtomicReference. This would avoid needing two volatile > reads to get to the data, and mean that "sharing" these (per above) is a bit > safer. The ChildData byte[] AtomicReference is only used by > PathChildrenCache.clearDataBytes() (not currently used by TreeCache at all), > and that capability could be easily maintained by having PathChildrenCache > use it's own simple subclass of ChildData containing the atomic reference. > If similar capability were to be added to TreeCache, I'd suggest it would be > better to just replace the node's ChildData object with a copy that has the > byte[] field nulled out (but same stat ref). > I'm fairly new to the code so apologies if there's something I've > missed/misunderstood! But if there is agreement, I'd also be happy to prepare > a PR. > Regards, > Nick -- This message was sent by Atlassian JIRA (v6.3.4#6332)