[ 
https://issues.apache.org/jira/browse/CURATOR-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15121849#comment-15121849
 ] 

Scott Blum commented on CURATOR-294:
------------------------------------

It just seems that not guaranteeing consistency of the stat and data in the 
returned ChildData objects restricts the usefulness of the cache, since you 
would typically have to get the data/stat again directly before doing any 
conditional update to a znode.

Ignoring the less-important suggestion for simplifying ChildData itself, 
wouldn't there still be significant value in a simple change to move the 
TreeNode data and stat atomic refs into a single ChildData atomic ref?

Cheers,
Nick

> 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
>
> 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)

Reply via email to