You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2015/04/27 21:51:53 UTC
curator git commit: Use sync instead of locks. It's simpler and
clearer
Repository: curator
Updated Branches:
refs/heads/CURATOR-164 f8b67dc42 -> 1a16f82ba
Use sync instead of locks. It's simpler and clearer
Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/1a16f82b
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/1a16f82b
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/1a16f82b
Branch: refs/heads/CURATOR-164
Commit: 1a16f82baae0360a2823db2cc811fbeb3d6e1392
Parents: f8b67dc
Author: randgalt <ra...@apache.org>
Authored: Mon Apr 27 14:51:46 2015 -0500
Committer: randgalt <ra...@apache.org>
Committed: Mon Apr 27 14:51:46 2015 -0500
----------------------------------------------------------------------
.../curator/x/discovery/details/Holder.java | 106 ++++---------------
.../discovery/details/ServiceDiscoveryImpl.java | 24 +----
2 files changed, 26 insertions(+), 104 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/curator/blob/1a16f82b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java
index cbc6236..d088f8d 100644
--- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java
+++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java
@@ -2,8 +2,6 @@ package org.apache.curator.x.discovery.details;
import org.apache.curator.framework.recipes.cache.NodeCache;
import org.apache.curator.x.discovery.ServiceInstance;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
class Holder<T>
{
@@ -18,7 +16,6 @@ class Holder<T>
private NodeCache cache;
private State state;
private long stateChangeMs;
- private final ReentrantLock lock = new ReentrantLock();
Holder(ServiceInstance<T> service, NodeCache nodeCache)
{
@@ -27,110 +24,49 @@ class Holder<T>
setState(State.NEW);
}
- ServiceInstance<T> getService()
+ synchronized ServiceInstance<T> getService()
{
- lock.lock();
- try
- {
- return service;
- }
- finally
- {
- lock.unlock();
- }
+ return service;
}
- ServiceInstance<T> getServiceIfRegistered()
+ synchronized ServiceInstance<T> getServiceIfRegistered()
{
- lock.lock();
- try
- {
- return (state == State.REGISTERED) ? service : null;
- }
- finally
- {
- lock.unlock();
- }
+ return (state == State.REGISTERED) ? service : null;
}
- void setService(ServiceInstance<T> service)
+ synchronized void setService(ServiceInstance<T> service)
{
- lock.lock();
- try
- {
- this.service = service;
- }
- finally
- {
- lock.unlock();
- }
+ this.service = service;
}
- NodeCache getAndClearCache()
+ synchronized NodeCache getAndClearCache()
{
- lock.lock();
- try
- {
- NodeCache localCache = cache;
- cache = null;
- return localCache;
- }
- finally
- {
- lock.unlock();
- }
+ NodeCache localCache = cache;
+ cache = null;
+ return localCache;
}
- boolean isRegistered()
+ synchronized boolean isRegistered()
{
- lock.lock();
- try
- {
- return state == State.REGISTERED;
- }
- finally
- {
- lock.unlock();
- }
+ return state == State.REGISTERED;
}
- boolean isLapsedUnregistered(int cleanThresholdMs)
+ synchronized boolean isLapsedUnregistered(int cleanThresholdMs)
{
- lock.lock();
- try
+ if ( state == State.UNREGISTERED )
{
- if ( state == State.UNREGISTERED )
+ long elapsed = System.currentTimeMillis() - stateChangeMs;
+ if ( elapsed >= cleanThresholdMs )
{
- long elapsed = System.currentTimeMillis() - stateChangeMs;
- if ( elapsed >= cleanThresholdMs )
- {
- return true;
- }
+ return true;
}
- return false;
- }
- finally
- {
- lock.unlock();
- }
- }
-
- void setState(State state)
- {
- lock.lock();
- try
- {
- this.state = state;
- stateChangeMs = System.currentTimeMillis();
- }
- finally
- {
- lock.unlock();
}
+ return false;
}
- Lock getLock()
+ synchronized void setState(State state)
{
- return lock;
+ this.state = state;
+ stateChangeMs = System.currentTimeMillis();
}
}
http://git-wip-us.apache.org/repos/asf/curator/blob/1a16f82b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java
index 8e3e1f9..a35cd3a 100644
--- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java
+++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java
@@ -59,6 +59,7 @@ import java.util.concurrent.atomic.AtomicLong;
/**
* A mechanism to register and query service instances using ZooKeeper
*/
+@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
{
private final Logger log = LoggerFactory.getLogger(getClass());
@@ -181,9 +182,8 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
{
clean();
- Holder<T> holder = getOrMakeHolder(service, null);
- holder.getLock().lock();
- try
+ final Holder<T> holder = getOrMakeHolder(service, null);
+ synchronized(holder)
{
if ( !holder.isRegistered() )
{
@@ -195,10 +195,6 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
String path = pathForInstance(service.getName(), service.getId());
client.setData().forPath(path, bytes);
}
- finally
- {
- holder.getLock().unlock();
- }
}
@VisibleForTesting
@@ -459,18 +455,13 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
{
for ( final Holder<T> holder : services.values() )
{
- holder.getLock().lock();
- try
+ synchronized(holder)
{
if ( holder.isRegistered() )
{
internalRegisterService(holder.getService());
}
}
- finally
- {
- holder.getLock().unlock();
- }
}
}
@@ -544,8 +535,7 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
{
if ( holder != null )
{
- holder.getLock().lock();
- try
+ synchronized(holder)
{
holder.setState(Holder.State.UNREGISTERED);
NodeCache cache = holder.getAndClearCache();
@@ -565,10 +555,6 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
// ignore
}
}
- finally
- {
- holder.getLock().unlock();
- }
}
}
}