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

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_r18874097
  
    --- Diff: 
curator-recipes/src/main/java/org/apache/curator/framework/recipes/shared/SharedValue.java
 ---
    @@ -129,29 +127,19 @@ public void setValue(byte[] newValue) throws Exception
          * value is updated. i.e. if the value is not successful you can get 
the updated value
          * by calling {@link #getValue()}.
          *
    +     * @deprecated use {@link #trySetValue(VersionedValue, byte[])} for 
stronger atomicity
    +     * guarantees. Even if this object's internal state is up-to-date, the 
caller has no way to
    +     * ensure that they've read the most recently seen value.
    +     *
          * @param newValue the new value to attempt
          * @return true if the change attempt was successful, false if not. If 
the change
          * was not successful, {@link #getValue()} will return the updated 
value
          * @throws Exception ZK errors, interruptions, etc.
          */
    +    @Deprecated
         public boolean trySetValue(byte[] newValue) throws Exception
         {
    -        Preconditions.checkState(state.get() == State.STARTED, "not 
started");
    -
    -        try
    -        {
    -            VersionedValue<byte[]> localCopy = currentValue.get();
    -            
client.setData().withVersion(localCopy.getVersion()).forPath(path, newValue);
    -            currentValue.set(new 
VersionedValue<byte[]>(localCopy.getVersion() + 1, Arrays.copyOf(newValue, 
newValue.length)));
    -            return true;
    -        }
    -        catch ( KeeperException.BadVersionException ignore )
    -        {
    -            // ignore
    -        }
    -
    -        readValue();
    -        return false;
    +        return trySetValue(currentValue.get(), newValue);
         }
    --- End diff --
    
    For more reference, check out [5 Things You Didn’t Know About 
Synchronization in Java and 
Scala](http://www.takipiblog.com/5-things-you-didnt-know-about-synchronization-in-java-and-scala/).
    
    "If the CAS fails the JVM will perform one round of spin locking where the 
thread parks to effectively put it to sleep between retrying the CAS. If these 
initial attempts fail (signaling a fairly higher level of contention for the 
lock) the  thread will move itself to a blocked state and enqueue itself in the 
list of threads vying for the lock and begin a series of spin locks."
    
    I'm essentially doing in code what the synchronization did internally.  
That's why I didn't have to rewrite any test code other than for the API change 
on the new method that was recently added.


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

Reply via email to