curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CURATOR-294) Optimize TreeCache, fix possible concurrency issue
Date Sat, 30 Jan 2016 03:49:39 GMT

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

ASF GitHub Bot commented on CURATOR-294:
----------------------------------------

GitHub user dragonsinth opened a pull request:

    https://github.com/apache/curator/pull/128

    CURATOR-294: Optimize TreeCache, fix possible concurrency issue

    

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/128.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 #128
    
----
commit 5e995ed7a315fba9f1327706735a7f8c1b417a83
Author: Scott Blum <dragonsinth@apache.org>
Date:   2016-01-28T17:38:57Z

    CURATOR-294: Make ChildData immutable; PathChildrenCache replaces instead of mutates.

commit b3e3fcc4848beaa148fc3a5b437e4b19b6e7f325
Author: Scott Blum <dragonsinth@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)

Mime
View raw message