hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mbau...@apache.org
Subject svn commit: r1350850 - /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Date Sat, 16 Jun 2012 03:13:04 GMT
Author: mbautin
Date: Sat Jun 16 03:13:03 2012
New Revision: 1350850

URL: http://svn.apache.org/viewvc?rev=1350850&view=rev
Log:
[master] remove serversToServerInfo shutdown

Author: pkhemani

Summary:
serversToServersInfo is a concurrent-hash-map. Access to it doesn't have to be synchronized.

I have removed all sync calls to serversToServerInfo except when master is shutting down and
waiting for serversToServerInfo to be cleaned up. (I will mark that place)

Test Plan:
will retest rolling restart.

but since this change relaxes locking, correctness has to be established by code review

Reviewers: liyintang, kranganathan, kannan, aaiyer

Reviewed By: kannan

CC: hbase-eng@, mbautin

Differential Revision: https://phabricator.fb.com/D495995

Task ID: 1120663

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java?rev=1350850&r1=1350849&r2=1350850&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
Sat Jun 16 03:13:03 2012
@@ -363,11 +363,9 @@ public class ServerManager {
         LOG.debug("region server race condition detected: " +
             info.getServerName());
       }
-
-      synchronized (this.serversToServerInfo) {
-        processServerInfoOnShutdown(info.getServerName(), true);
-        notifyServers();
-      }
+      
+      processServerInfoOnShutdown(info.getServerName(), true);
+      notifyServers();
 
       return new HMsg[] {HMsg.REGIONSERVER_STOP};
     } else {
@@ -420,48 +418,46 @@ public class ServerManager {
    * @param msgs
    */
   private void processRegionServerExit(HServerInfo serverInfo, HMsg[] msgs) {
-    synchronized (this.serversToServerInfo) {
-      // This method removes ROOT/META from the list and marks them to be
-      // reassigned in addition to other housework.
-      if (processServerInfoOnShutdown(serverInfo.getServerName(), false)) {
-        // Only process the exit message if the server still has registered info.
-        // Otherwise we could end up processing the server exit twice.
-        LOG.info("Region server " + serverInfo.getServerName() +
+    // This method removes ROOT/META from the list and marks them to be
+    // reassigned in addition to other housework.
+    if (processServerInfoOnShutdown(serverInfo.getServerName(), false)) {
+      // Only process the exit message if the server still has registered info.
+      // Otherwise we could end up processing the server exit twice.
+      LOG.info("Region server " + serverInfo.getServerName() +
           ": MSG_REPORT_EXITING");
-        // Get all the regions the server was serving reassigned
-        // (if we are not shutting down).
-        if (!master.isClosed()) {
-          for (int i = 1; i < msgs.length; i++) {
-            LOG.info("Processing " + msgs[i] + " from " +
+      // Get all the regions the server was serving reassigned
+      // (if we are not shutting down).
+      if (!master.isClosed()) {
+        for (int i = 1; i < msgs.length; i++) {
+          LOG.info("Processing " + msgs[i] + " from " +
               serverInfo.getServerName());
-            assert msgs[i].getType() == HMsg.Type.MSG_REGION_CLOSE;
-            HRegionInfo info = msgs[i].getRegionInfo();
-            // Meta/root region offlining is handed in removeServerInfo above.
-            if (!info.isMetaRegion()) {
-              synchronized (master.getRegionManager()) {
-                if (!master.getRegionManager().isOfflined(info.getRegionNameAsString()))
{
-                  master.getRegionManager().setUnassigned(info, true);
-                } else {
-                  master.getRegionManager().removeRegion(info);
-                }
+          assert msgs[i].getType() == HMsg.Type.MSG_REGION_CLOSE;
+          HRegionInfo info = msgs[i].getRegionInfo();
+          // Meta/root region offlining is handed in removeServerInfo above.
+          if (!info.isMetaRegion()) {
+            synchronized (master.getRegionManager()) {
+              if (!master.getRegionManager().isOfflined(info.getRegionNameAsString())) {
+                master.getRegionManager().setUnassigned(info, true);
+              } else {
+                master.getRegionManager().removeRegion(info);
               }
             }
           }
         }
-        // There should not be any regions in transition for this server - the
-        // server should finish transitions itself before closing
-        Map<String, RegionState> inTransition = master.getRegionManager()
-            .getRegionsInTransitionOnServer(serverInfo.getServerName());
-        for (Map.Entry<String, RegionState> entry : inTransition.entrySet()) {
-          LOG.warn("Region server " + serverInfo.getServerName()
-              + " shut down with region " + entry.getKey() + " in transition "
-              + "state " + entry.getValue());
-          master.getRegionManager().setUnassigned(entry.getValue().getRegionInfo(),
-              true);
-        }
-        LOG.info("Removing server's info " + serverInfo.getServerName());
-        this.serversToServerInfo.remove(serverInfo.getServerName());
       }
+      // There should not be any regions in transition for this server - the
+      // server should finish transitions itself before closing
+      Map<String, RegionState> inTransition = master.getRegionManager()
+          .getRegionsInTransitionOnServer(serverInfo.getServerName());
+      for (Map.Entry<String, RegionState> entry : inTransition.entrySet()) {
+        LOG.warn("Region server " + serverInfo.getServerName()
+            + " shut down with region " + entry.getKey() + " in transition "
+            + "state " + entry.getValue());
+        master.getRegionManager().setUnassigned(entry.getValue().getRegionInfo(),
+            true);
+      }
+      LOG.info("Removing server's info " + serverInfo.getServerName());
+      this.serversToServerInfo.remove(serverInfo.getServerName());
     }
   }
 
@@ -921,9 +917,7 @@ public class ServerManager {
    * @return Read-only map of servers to serverinfo.
    */
   public Map<String, HServerInfo> getServersToServerInfo() {
-    synchronized (this.serversToServerInfo) {
-      return Collections.unmodifiableMap(this.serversToServerInfo);
-    }
+    return Collections.unmodifiableMap(this.serversToServerInfo);
   }
 
   /**
@@ -932,11 +926,9 @@ public class ServerManager {
    * if nothing found.
    */
   public HServerInfo getHServerInfo(final HServerAddress hsa) {
-    synchronized(this.serversToServerInfo) {
-      // TODO: This is primitive.  Do a better search.
-      for (Map.Entry<String, HServerInfo> e: this.serversToServerInfo.entrySet()) {
-        if (e.getValue().getServerAddress().equals(hsa)) return e.getValue();
-      }
+    // TODO: This is primitive.  Do a better search.
+    for (Map.Entry<String, HServerInfo> e: this.serversToServerInfo.entrySet()) {
+      if (e.getValue().getServerAddress().equals(hsa)) return e.getValue();
     }
     return null;
   }
@@ -945,9 +937,7 @@ public class ServerManager {
    * @return Read-only map of servers to load.
    */
   public Map<String, HServerLoad> getServersToLoad() {
-    synchronized (this.serversToLoad) {
-      return Collections.unmodifiableMap(serversToLoad);
-    }
+    return Collections.unmodifiableMap(serversToLoad);
   }
 
   /**
@@ -960,16 +950,16 @@ public class ServerManager {
   }
 
   /**
+   * There is no way to guarantee that the returned servers are really online
+   *
    * @return the list of the HServerAddress for all the online region servers.
    */
   public List<HServerAddress> getOnlineRegionServerList() {
-    synchronized(this.serversToServerInfo) {
-      List<HServerAddress> serverList = new ArrayList<HServerAddress>();
-      for (HServerInfo serverInfo: this.serversToServerInfo.values()) {
-        serverList.add(serverInfo.getServerAddress());
-      }
-      return serverList;
+    List<HServerAddress> serverList = new ArrayList<HServerAddress>();
+    for (HServerInfo serverInfo: this.serversToServerInfo.values()) {
+      serverList.add(serverInfo.getServerAddress());
     }
+    return serverList;
   }
 
   /**



Mime
View raw message