geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aba...@apache.org
Subject [29/50] [abbrv] incubator-geode git commit: Performance enhancements ported from GemFire 8.2.0.x
Date Mon, 22 Aug 2016 16:39:57 GMT
Performance enhancements ported from GemFire 8.2.0.x

This increases throughput in GMSMembershipManager by avoiding write-locks
on the view in some places.

I also found GMSHealthMonitor was mistakenly referencing a CORBA class's "debug"
static variable and found an out-of-date comment in AnalyzeSerializablesJUnitTest.


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/3f6acdc7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/3f6acdc7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/3f6acdc7

Branch: refs/heads/master
Commit: 3f6acdc70fd6454af6b44a719a318e2106d68e46
Parents: 3e20193
Author: Bruce Schuchardt <bschuchardt@pivotal.io>
Authored: Tue Jul 19 13:31:09 2016 -0700
Committer: Bruce Schuchardt <bschuchardt@pivotal.io>
Committed: Tue Jul 19 13:31:09 2016 -0700

----------------------------------------------------------------------
 .../membership/gms/fd/GMSHealthMonitor.java     |   8 +-
 .../gms/mgr/GMSMembershipManager.java           | 191 ++++++++++---------
 .../AnalyzeSerializablesJUnitTest.java          |   4 +-
 3 files changed, 109 insertions(+), 94 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3f6acdc7/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
old mode 100755
new mode 100644
index e0f22a5..586427f
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
@@ -295,9 +295,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler
{
           socket.shutdownOutput();
           GMSHealthMonitor.this.stats.incFinalCheckResponsesSent();
           GMSHealthMonitor.this.stats.incTcpFinalCheckResponsesSent();
-          if (debug) {
-            logger.debug("HealthMonitor: server replied OK.");
-          }
+          logger.debug("HealthMonitor: server replied OK.");
         } else {
           if (logger.isDebugEnabled()) {
             logger.debug("HealthMonitor: sending ERROR reply - my UUID is {},{} received
is {},{}.  My viewID is {} received is {}", 
@@ -312,9 +310,7 @@ public class GMSHealthMonitor implements HealthMonitor, MessageHandler
{
           socket.shutdownOutput();
           GMSHealthMonitor.this.stats.incFinalCheckResponsesSent();
           GMSHealthMonitor.this.stats.incTcpFinalCheckResponsesSent();
-          if (debug) {
-            logger.debug("HealthMonitor: server replied ERROR.");
-          }
+          logger.debug("HealthMonitor: server replied ERROR.");
         }
       } catch (IOException e) {
         // this is expected if it is a connection-timeout or other failure

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3f6acdc7/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
index b755f6c..e6ca8e1 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/mgr/GMSMembershipManager.java
@@ -60,8 +60,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.locks.*;
 
 public class GMSMembershipManager implements MembershipManager, Manager
 {
@@ -219,7 +218,9 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * @see #latestView
    */
   protected ReadWriteLock latestViewLock = new ReentrantReadWriteLock();
-  
+  private final Lock latestViewReadLock = latestViewLock.readLock();
+  private final Lock latestViewWriteLock = latestViewLock.writeLock();
+
   /**
    * This is the listener that accepts our membership events
    */
@@ -424,7 +425,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
 //     }
     // We perform the update under a global lock so that other
     // incoming events will not be lost in terms of our global view.
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       // first determine the version for multicast message serialization
       Version version = Version.CURRENT;
@@ -597,7 +598,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
       catch (DistributedSystemDisconnectedException se) {
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
 
@@ -630,7 +631,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
     services.setShutdownCause(null);
     services.getCancelCriterion().cancel(null);
     
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       try {
         this.isJoining = true; // added for bug #44373
@@ -668,7 +669,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
       }
     }
     finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
 
@@ -880,7 +881,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
     final InternalDistributedMember member = (InternalDistributedMember)dm;
     boolean warn = false;
     
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       // At this point, the join may have been discovered by
       // other means.
@@ -955,7 +956,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         latestView = newMembers;
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
     if (warn) { // fix for bug #41538 - deadlock while alerting
       logger.warn(LocalizedMessage.create(
@@ -969,7 +970,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
 
   /** starts periodic task to perform cleanup chores such as expire surprise members */
   private void startCleanupTimer() {
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (this.cleanupTimer != null) {
         return;
@@ -980,7 +981,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         SystemTimer.SystemTimerTask st = new SystemTimer.SystemTimerTask() {
           @Override
           public void run2() {
-            latestViewLock.writeLock().lock();
+            latestViewWriteLock.lock();
             try {
               long oldestAllowed = System.currentTimeMillis() - surpriseMemberTimeout;
               for (Iterator it=surpriseMembers.entrySet().iterator(); it.hasNext(); ) {
@@ -996,14 +997,14 @@ public class GMSMembershipManager implements MembershipManager, Manager
                 }
               }
             } finally {
-              latestViewLock.writeLock().unlock();
+              latestViewWriteLock.unlock();
             }
           }
         };
         this.cleanupTimer.scheduleAtFixedRate(st, surpriseMemberTimeout, surpriseMemberTimeout/3);
       } // ds != null && ds.isConnected()
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
   /**
@@ -1029,7 +1030,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
   }
   
   public void warnShun(DistributedMember m) {
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (!shunnedMembers.containsKey(m)) {
         return; // not shunned
@@ -1039,7 +1040,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
       }
       shunnedAndWarnedMembers.add(m);
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
     // issue warning outside of sync since it may cause messaging and we don't
     // want to hold the view lock while doing that
@@ -1063,47 +1064,48 @@ public class GMSMembershipManager implements MembershipManager, Manager
     InternalDistributedMember m = msg.getSender();
     boolean shunned = false;
 
-    // First grab the lock: check the sender against our stabilized view.
-    latestViewLock.writeLock().lock();
-    try {
-      if (isShunned(m)) {
-        if (msg instanceof StartupMessage) {
-          endShun(m);
-        }
-        else {
-          // fix for bug 41538 - sick alert listener causes deadlock
-          // due to view lock being held during messaging
-          shunned = true;
+    // If this member is shunned or new, grab the latestViewWriteLock: update the appropriate
data structure.
+    //    synchronized (latestViewLock) {
+    if (isShunnedOrNew(m)) {
+      latestViewWriteLock.lock();
+      try {
+        if (isShunned(m)) {
+          if (msg instanceof StartupMessage) {
+            endShun(m);
+          } else {
+            // fix for bug 41538 - sick alert listener causes deadlock
+            // due to view latestViewReadWriteLock being held during messaging
+            shunned = true;
+          }
         }
-      } // isShunned
 
-      if (!shunned) {
-        isNew = !latestView.contains(m) && !surpriseMembers.containsKey(m);
-
-        // If it's a new sender, wait our turn, generate the event
-        if (isNew) {
-          shunned = !addSurpriseMember(m);
-        } // isNew
+        if (!shunned) {
+          // If it's a new sender, wait our turn, generate the event
+          if (isNew(m)) {
+            shunned = !addSurpriseMember(m);
+          }
+        }
+      } finally {
+        latestViewWriteLock.unlock();
       }
-
-      // Latch the view before we unlock
-    } finally {
-      latestViewLock.writeLock().unlock();
     }
-    
+
     if (shunned) { // bug #41538 - shun notification must be outside synchronization to avoid
hanging
       warnShun(m);
-      logger.info("Membership: Ignoring message from shunned member <{}>:{}", m, msg);
+      if (logger.isTraceEnabled(LogMarker.DISTRIBUTION_VIEWS)) {
+        logger.trace(LogMarker.DISTRIBUTION_VIEWS, "Membership: Ignoring message from shunned
member <{}>:{}", m, msg);
+      }
       throw new MemberShunnedException(m);
     }
-    
+
     listener.messageReceived(msg);
   }
 
-  /**
-   * Process a new view object, or place on the startup queue
-   * @param viewArg the new view
-   */
+
+    /**
+     * Process a new view object, or place on the startup queue
+     * @param viewArg the new view
+     */
   protected void handleOrDeferViewEvent(NetView viewArg) {
     if (this.isJoining) {
       // bug #44373 - queue all view messages while joining.
@@ -1114,7 +1116,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         return;
       }
     }
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       synchronized(startupLock) {
         if (!processingEvents) {
@@ -1131,7 +1133,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
 
       listener.messageReceived(v);
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
   
@@ -1146,7 +1148,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * @param suspectInfo the suspectee and suspector
    */
   protected void handleOrDeferSuspect(SuspectMember suspectInfo) {
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       synchronized(startupLock) {
         if (!processingEvents) {
@@ -1163,7 +1165,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         // let's not get huffy about it
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
 
@@ -1325,9 +1327,9 @@ public class GMSMembershipManager implements MembershipManager, Manager
     // Grab the latest view under a mutex...
     NetView v;
 
-    latestViewLock.readLock().lock();
+    latestViewReadLock.lock();
     v = latestView;
-    latestViewLock.readLock().unlock();
+    latestViewReadLock.unlock();
 
     NetView result = new NetView(v, v.getViewId());
     
@@ -1348,11 +1350,11 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * @return the lead member associated with the latest view
    */
   public DistributedMember getLeadMember() {
-    latestViewLock.readLock().lock();
+    latestViewReadLock.lock();
     try {
       return latestView == null? null : latestView.getLeadMember();
     } finally {
-      latestViewLock.readLock().unlock();
+      latestViewReadLock.unlock();
     }
   }
   
@@ -1365,18 +1367,18 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * @return the current membership view coordinator
    */
   public DistributedMember getCoordinator() {
-    latestViewLock.readLock().lock();
+    latestViewReadLock.lock();
     try {
       return latestView == null? null : latestView.getCoordinator();
     } finally {
-      latestViewLock.readLock().unlock();
+      latestViewReadLock.unlock();
     }
   }
 
   public boolean memberExists(DistributedMember m) {
-    latestViewLock.readLock().lock();
+    latestViewReadLock.lock();
     NetView v = latestView;
-    latestViewLock.readLock().unlock();
+    latestViewReadLock.unlock();
     return v.getMembers().contains(m);
   }
   
@@ -1500,11 +1502,11 @@ public class GMSMembershipManager implements MembershipManager, Manager
         // Make sure that channel information is consistent
         // Probably not important in this particular case, but just
         // to be consistent...
-        latestViewLock.writeLock().lock();
+        latestViewWriteLock.lock();
         try {
           destroyMember(address, false, "orderly shutdown");
         } finally {
-          latestViewLock.writeLock().unlock();
+          latestViewWriteLock.unlock();
         }
       }
     }
@@ -1684,13 +1686,13 @@ public class GMSMembershipManager implements MembershipManager, Manager
     InternalDistributedMember[] keys;
     if (content.forAll()) {
       allDestinations = true;
-      latestViewLock.writeLock().lock();
+      latestViewReadLock.lock();
       try {
         List<InternalDistributedMember> keySet = latestView.getMembers();
         keys = new InternalDistributedMember[keySet.size()];
         keys = (InternalDistributedMember[])keySet.toArray(keys);
       } finally {
-        latestViewLock.writeLock().unlock();
+        latestViewReadLock.unlock();
       }
     }
     else {
@@ -1917,11 +1919,11 @@ public class GMSMembershipManager implements MembershipManager, Manager
       if (m != null) {
         GMSMember id = (GMSMember)m.getNetMember();
         if (!id.hasUUID()) {
-          latestViewLock.readLock().lock();
+          latestViewReadLock.lock();
           try {
             addresses[i] = latestView.getCanonicalID(addresses[i]);
           } finally {
-            latestViewLock.readLock().unlock();
+            latestViewReadLock.unlock();
           }
         }
       }
@@ -1941,9 +1943,9 @@ public class GMSMembershipManager implements MembershipManager, Manager
   
   public void setShutdown()
   {
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     shutdownInProgress = true;
-    latestViewLock.writeLock().unlock();
+    latestViewWriteLock.unlock();
   }
 
   @Override
@@ -1965,7 +1967,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
       boolean crashed, final String reason) {
     
     // Make sure it is removed from the view
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (latestView.contains(member)) {
         NetView newView = new NetView(latestView, latestView.getViewId());
@@ -1973,7 +1975,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         latestView = newView;
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
     
     surpriseMembers.remove(member);
@@ -2031,29 +2033,46 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * list if it was shunned too far in the past.
    * 
    * Concurrency: protected by {@link #latestViewLock} ReentrantReadWriteLock
-   * 
+   *
+   * @guarded.By latestViewLock
    * @return true if the given member is a zombie
    */
   public boolean isShunned(DistributedMember m) {
-    latestViewLock.writeLock().lock();
+    if (!shunnedMembers.containsKey(m)) {
+      return false;
+    }
+
+    latestViewWriteLock.lock();
     try {
-      if (!shunnedMembers.containsKey(m))
-        return false;
-      
       // Make sure that the entry isn't stale...
       long shunTime = ((Long)shunnedMembers.get(m)).longValue();
       long now = System.currentTimeMillis();
-      if (shunTime + SHUNNED_SUNSET * 1000 > now)
+      if (shunTime + SHUNNED_SUNSET * 1000 > now) {
         return true;
+      }
       
       // Oh, it _is_ stale.  Remove it while we're here.
       endShun(m);
       return false;
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
 
+  private final boolean isShunnedOrNew(final InternalDistributedMember m) {
+    latestViewReadLock.lock();
+    try {
+      return shunnedMembers.containsKey(m) || isNew(m);
+    } finally { // synchronized
+      latestViewReadLock.unlock();
+    }
+  }
+
+  // must be invoked under view read or write lock
+  private final boolean isNew(final InternalDistributedMember m) {
+    return !latestView.contains(m) && !surpriseMembers.containsKey(m);
+  }
+
   /**
    * Indicate whether the given member is in the surprise member list
    * <P>
@@ -2068,7 +2087,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
    * @return true if the given member is a surprise member
    */
   public boolean isSurpriseMember(DistributedMember m) {
-    latestViewLock.readLock().lock();
+    latestViewReadLock.lock();
     try  {
       if (surpriseMembers.containsKey(m)) {
         long birthTime = ((Long)surpriseMembers.get(m)).longValue();
@@ -2077,7 +2096,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
       }
       return false;
     } finally {
-      latestViewLock.readLock().unlock();
+      latestViewReadLock.unlock();
     }
   }
   
@@ -2091,11 +2110,11 @@ public class GMSMembershipManager implements MembershipManager, Manager
     if (logger.isDebugEnabled()) {
       logger.debug("test hook is adding surprise member {} birthTime={}", m, birthTime);
     }
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       surpriseMembers.put((InternalDistributedMember)m, Long.valueOf(birthTime));
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
   
@@ -2253,11 +2272,11 @@ public class GMSMembershipManager implements MembershipManager, Manager
         }
       }
       if (!wait) {
-        latestViewLock.readLock().lock();
+        latestViewReadLock.lock();
         try {
           wait = this.latestView.contains(idm);
         } finally {
-          latestViewLock.readLock().unlock();
+          latestViewReadLock.unlock();
         }
         if (wait && logger.isDebugEnabled()) {
           logger.debug("waiting for {} to leave the membership view", mbr);
@@ -2310,7 +2329,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
     CountDownLatch currentLatch = null;
     // ARB: preconditions
     // remoteId != null
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (latestView == null) {
         // Not sure how this would happen, but see bug 38460.
@@ -2326,7 +2345,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         this.memberLatch.put(remoteId, currentLatch);
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
 
     if (!foundRemoteId) {
@@ -2381,7 +2400,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
   
   public void registerTestHook(MembershipTestHook mth) {
     // lock for additions to avoid races during startup
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (this.membershipTestHooks == null) {
         this.membershipTestHooks = Collections.singletonList(mth);
@@ -2392,12 +2411,12 @@ public class GMSMembershipManager implements MembershipManager, Manager
         this.membershipTestHooks = l;
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
   
   public void unregisterTestHook(MembershipTestHook mth) {
-    latestViewLock.writeLock().lock();
+    latestViewWriteLock.lock();
     try {
       if (this.membershipTestHooks != null) {
         if (this.membershipTestHooks.size() == 1) {
@@ -2409,7 +2428,7 @@ public class GMSMembershipManager implements MembershipManager, Manager
         }
       }
     } finally {
-      latestViewLock.writeLock().unlock();
+      latestViewWriteLock.unlock();
     }
   }
   

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/3f6acdc7/geode-core/src/test/java/com/gemstone/gemfire/codeAnalysis/AnalyzeSerializablesJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/codeAnalysis/AnalyzeSerializablesJUnitTest.java
b/geode-core/src/test/java/com/gemstone/gemfire/codeAnalysis/AnalyzeSerializablesJUnitTest.java
index 1c9b8c4..d551c9f 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/codeAnalysis/AnalyzeSerializablesJUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/codeAnalysis/AnalyzeSerializablesJUnitTest.java
@@ -53,9 +53,9 @@ public class AnalyzeSerializablesJUnitTest {
   @Before
   public void loadClasses() throws Exception {
     String version = System.getProperty("java.runtime.version");
-    boolean jdk17 = version != null && version.startsWith("1.8");
+    boolean jdk18 = version != null && version.startsWith("1.8");
       // sanctioned info is based on a 1.7 compiler
-    Assume.assumeTrue("AnalyzeSerializables requires a Java 7 but tests are running with
v"+version, jdk17);
+    Assume.assumeTrue("AnalyzeSerializables requires a Java 8 but tests are running with
v"+version, jdk18);
     if (classes.size() > 0) {
       return;
     }


Mime
View raw message