Mailing-List: contact dev-help@curator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@curator.apache.org
From: dragonsinth <git@git.apache.org>
To: dev@curator.apache.org
Reply-To: dev@curator.apache.org
References: <git-pr-47-curator@git.apache.org>
In-Reply-To: <git-pr-47-curator@git.apache.org>
Subject: [GitHub] curator pull request: CURATOR-151: SharedValue/SharedCount
 API upd...
Content-Type: text/plain
Message-Id: <20141015041818.32FE18A14F0@tyr.zones.apache.org>
Date: Wed, 15 Oct 2014 04:18:18 +0000 (UTC)

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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---