Mailing-List: contact dev-help@curator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@curator.apache.org
Date: Thu, 9 Oct 2014 17:11:34 +0000 (UTC)
From: "ASF GitHub Bot (JIRA)" <jira@apache.org>
To: dev@curator.apache.org
Message-ID: <JIRA.12745544.1412269699000.229214.1412874694538@Atlassian.JIRA>
In-Reply-To: <JIRA.12745544.1412269699000@Atlassian.JIRA>
References: <JIRA.12745544.1412269699000@Atlassian.JIRA>
 <JIRA.12745544.1412269699662@arcas>
Subject: [jira] [Commented] (CURATOR-151) SharedValue has limited utility
 but can be improved
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

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

Github user dragonsinth commented on a diff in the pull request:

    https://github.com/apache/curator/pull/47#discussion_r18659175
  
    --- Diff: curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java ---
    @@ -184,6 +178,25 @@ public boolean trySetValue(VersionedValue<byte[]> newValue) throws Exception
             return false;
         }
     
    +    private void updateValue(int version, byte[] bytes)
    +    {
    +        while (true)
    +        {
    +            VersionedValue<byte[]> current = currentValue.get();
    +            if (current.getVersion() >= version)
    +            {
    +                // A newer version was concurrently set.
    +                return;
    +            }
    +            if ( currentValue.compareAndSet(current, new VersionedValue<byte[]>(version, bytes)) )
    +            {
    +                // Successfully set.
    +                return;
    +            }
    +            // Lost a race, retry.
    +        }
    +    }
    +
         /**
    --- End diff --
    
    Let me elaborate a little on the race condition this solves.
    
    Imagine the current value is (1, A) you have one thread calling trySetValue(B) another calling trySetValue(C) at the same time.  Imagine thread 1 succeeds in making the server call first and updating the server value to (2, B) and immediately afterwards thread 2 succeeds in calling the server and updating the server value to (3, C).   But to due to thread scheduling or what have you, both threads actually enter updateValue() at the same time.  How do we ensure that the correct value ends up in currentValue when both threads exit updateValue so that the client has the correct server state?
    
    That's what the compare-and-set loop solves.  Imagine both threads enter the loop and initially read current as (1, A).  Then they both try to compare and set (1, A) to either (2, B) or (3, C).  Only one of them can win the race.  If (2, B) gets set, then the first thread will return and the second thread will try again, and this time succeed in updating (2, B) -> (3, C).  If (3, C) gets set then the second thread will return, and the first thread will try again.  But this time it will exit out because it will see that someone else already set a higher-versioned value.
    
    This actually solves several potential races, between threads calling setValue/trySetValue at the same time and also any threads that could be calling readValue() at the same time (watcher, failed trySetValue(), or even the start() call).


> SharedValue has limited utility but can be improved
> ---------------------------------------------------
>
>                 Key: CURATOR-151
>                 URL: https://issues.apache.org/jira/browse/CURATOR-151
>             Project: Apache Curator
>          Issue Type: Improvement
>          Components: Recipes
>    Affects Versions: 2.6.0
>            Reporter: Jordan Zimmerman
>            Assignee: Jordan Zimmerman
>             Fix For: 2.7.0
>
>
> Currently, SharedValue has limited utility as the internally managed version is always used for trySetValue. A good improvement would be a) add an API to get the current value AND current version and b) add an alternate trySetValue that takes a new value AND an expected version. 


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)