curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Hill <apa...@nickhill.org>
Subject TreeCache question
Date Thu, 28 Jan 2016 02:42:41 GMT
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

Mime
View raw message