You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2009/05/18 20:42:13 UTC

svn commit: r776050 - in /hadoop/hbase/trunk: ./ src/java/org/apache/hadoop/hbase/master/

Author: stack
Date: Mon May 18 18:42:13 2009
New Revision: 776050

URL: http://svn.apache.org/viewvc?rev=776050&view=rev
Log:
HBASE-1421 Processing a regionserver message -- OPEN, CLOSE, SPLIT, etc. -- and if we're carrying more than one message in payload, if exception, all messages that follow are dropped on floor

Modified:
    hadoop/hbase/trunk/CHANGES.txt
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionManager.java
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionServerOperation.java
    hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ServerManager.java

Modified: hadoop/hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/CHANGES.txt?rev=776050&r1=776049&r2=776050&view=diff
==============================================================================
--- hadoop/hbase/trunk/CHANGES.txt (original)
+++ hadoop/hbase/trunk/CHANGES.txt Mon May 18 18:42:13 2009
@@ -133,6 +133,9 @@
                (Clint Morgan via Stack)
    HBASE-1431  NPE in HTable.checkAndSave when row doesn't exist (Guilherme
                Mauro Germoglio Barbosa via Andrew Purtell)
+   HBASE-1421  Processing a regionserver message -- OPEN, CLOSE, SPLIT, etc. --
+               and if we're carrying more than one message in payload, if
+               exception, all messages that follow are dropped on floor
 
   IMPROVEMENTS
    HBASE-1089  Add count of regions on filesystem to master UI; add percentage

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java?rev=776050&r1=776049&r2=776050&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java Mon May 18 18:42:13 2009
@@ -40,11 +40,9 @@
    * @param master
    * @param info
    * @param regionInfo
-   * @throws IOException
    */
   public ProcessRegionOpen(HMaster master, HServerInfo info,
-      HRegionInfo regionInfo)
-  throws IOException {
+      HRegionInfo regionInfo) {
     super(master, regionInfo);
     if (info == null) {
       throw new NullPointerException("HServerInfo cannot be null; " +

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionManager.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionManager.java?rev=776050&r1=776049&r2=776050&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionManager.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionManager.java Mon May 18 18:42:13 2009
@@ -1230,9 +1230,8 @@
      */
     synchronized void setPendingOpen(final String serverName) {
       if (!this.unassigned) {
-        throw new IllegalStateException(
-            "Cannot assign a region that is not currently unassigned. State: " +
-            toString());
+        LOG.warn("Cannot assign a region that is not currently unassigned. " +
+          "FIX!! State: " + toString());
       }
       this.unassigned = false;
       this.pendingOpen = true;
@@ -1250,9 +1249,8 @@
 
     synchronized void setOpen() {
       if (!pendingOpen) {
-        throw new IllegalStateException(
-            "Cannot set a region as open if it has not been pending. State: " +
-            toString());
+        LOG.warn("Cannot set a region as open if it has not been pending. " +
+          "FIX!! State: " + toString());
       }
       this.unassigned = false;
       this.pendingOpen = false;
@@ -1284,9 +1282,8 @@
 
     synchronized void setPendingClose() {
       if (!closing) {
-        throw new IllegalStateException(
-            "Cannot set a region as pending close if it has not been closing. " +
-            "State: " + toString());
+        LOG.warn("Cannot set a region as pending close if it has not been " +
+          "closing.  FIX!! State: " + toString());
       }
       this.unassigned = false;
       this.pendingOpen = false;
@@ -1353,4 +1350,4 @@
       return Bytes.compareTo(getRegionName(), o.getRegionName());
     }
   }
-}
+}
\ No newline at end of file

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionServerOperation.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionServerOperation.java?rev=776050&r1=776049&r2=776050&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionServerOperation.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/RegionServerOperation.java Mon May 18 18:42:13 2009
@@ -42,7 +42,7 @@
     // DelayQueue we're inserted in on lease expiration.
     this.expire = System.currentTimeMillis() + this.master.leaseTimeout / 2;
   }
-  
+
   public long getDelay(TimeUnit unit) {
     return unit.convert(this.expire - System.currentTimeMillis(),
       TimeUnit.MILLISECONDS);

Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ServerManager.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ServerManager.java?rev=776050&r1=776049&r2=776050&view=diff
==============================================================================
--- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ServerManager.java (original)
+++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/master/ServerManager.java Mon May 18 18:42:13 2009
@@ -388,26 +388,30 @@
     return processMsgs(serverInfo, mostLoadedRegions, msgs);
   }
 
-  /** 
+  /*
    * Process all the incoming messages from a server that's contacted us.
-   * 
    * Note that we never need to update the server's load information because
    * that has already been done in regionServerReport.
+   * @param serverInfo
+   * @param mostLoadedRegions
+   * @param incomingMsgs
+   * @return
    */
   private HMsg[] processMsgs(HServerInfo serverInfo,
-      HRegionInfo[] mostLoadedRegions, HMsg incomingMsgs[])
-  throws IOException { 
+      HRegionInfo[] mostLoadedRegions, HMsg incomingMsgs[]) { 
     ArrayList<HMsg> returnMsgs = new ArrayList<HMsg>();
     if (serverInfo.getServerAddress() == null) {
       throw new NullPointerException("Server address cannot be null; " +
         "hbase-958 debugging");
     }
     // Get reports on what the RegionServer did.
+    // Be careful that in message processors we don't throw exceptions that
+    // break the switch below because then we might drop messages on the floor.
     int openingCount = 0;
     for (int i = 0; i < incomingMsgs.length; i++) {
       HRegionInfo region = incomingMsgs[i].getRegionInfo();
       LOG.info("Received " + incomingMsgs[i] + " from " +
-          serverInfo.getServerName());
+        serverInfo.getServerName() + "; " + i + " of " + incomingMsgs.length);
       switch (incomingMsgs[i].getType()) {
         case MSG_REPORT_PROCESS_OPEN:
           openingCount++;
@@ -422,13 +426,11 @@
           break;
 
         case MSG_REPORT_SPLIT:
-          processSplitRegion(region, incomingMsgs[++i], incomingMsgs[++i],
-              returnMsgs);
+          processSplitRegion(region, incomingMsgs[++i], incomingMsgs[++i]);
           break;
 
         default:
-          throw new IOException(
-            "Impossible state during message processing. Instruction: " +
+          LOG.warn("Impossible state during message processing. Instruction: " +
             incomingMsgs[i].getType());
       }
     }
@@ -456,7 +458,7 @@
     return returnMsgs.toArray(new HMsg[returnMsgs.size()]);
   }
   
-  /**
+  /*
    * A region has split.
    *
    * @param region
@@ -464,21 +466,16 @@
    * @param splitB
    * @param returnMsgs
    */
-  private void processSplitRegion(HRegionInfo region, HMsg splitA, HMsg splitB,
-      ArrayList<HMsg> returnMsgs) {
-
+  private void processSplitRegion(HRegionInfo region, HMsg splitA, HMsg splitB) {
     synchronized (master.regionManager) {
       // Cancel any actions pending for the affected region.
       // This prevents the master from sending a SPLIT message if the table
       // has already split by the region server. 
       master.regionManager.endActions(region.getRegionName());
-
       HRegionInfo newRegionA = splitA.getRegionInfo();
       master.regionManager.setUnassigned(newRegionA, false);
-
       HRegionInfo newRegionB = splitB.getRegionInfo();
       master.regionManager.setUnassigned(newRegionB, false);
-
       if (region.isMetaTable()) {
         // A meta region has split.
         master.regionManager.offlineMetaRegion(region.getStartKey());
@@ -487,10 +484,14 @@
     }
   }
 
-  /** Region server is reporting that a region is now opened */
+  /*
+   * Region server is reporting that a region is now opened
+   * @param serverInfo
+   * @param region
+   * @param returnMsgs
+   */
   private void processRegionOpen(HServerInfo serverInfo, 
-    HRegionInfo region, ArrayList<HMsg> returnMsgs) 
-  throws IOException {
+      HRegionInfo region, ArrayList<HMsg> returnMsgs) {
     boolean duplicateAssignment = false;
     synchronized (master.regionManager) {
       if (!master.regionManager.isUnassigned(region) &&
@@ -549,19 +550,30 @@
           // Note that the table has been assigned and is waiting for the
           // meta table to be updated.
           master.regionManager.setOpen(region.getRegionNameAsString());
-          // Queue up an update to note the region location.
-          try {
-            master.toDoQueue.put(
-                new ProcessRegionOpen(master, serverInfo, region));
-          } catch (InterruptedException e) {
-            throw new RuntimeException(
-                "Putting into toDoQueue was interrupted.", e);
+          // Queue up an update to note the region location.  Do inside
+          // a retry loop in case interrupted.
+          boolean succeeded = false;
+          for (int i = 0; i < 10; i++) {
+            try {
+              master.toDoQueue.
+                put(new ProcessRegionOpen(master, serverInfo, region));
+              succeeded = true;
+            } catch (InterruptedException e) {
+              LOG.warn("Putting into toDoQueue was interrupted.", e);
+            }
+          }
+          if (!succeeded) {
+            LOG.warn("FAILED ADDING OPEN TO TODO QUEUE: " + serverInfo);
           }
-        } 
+        }
       }
     }
   }
-  
+
+  /*
+   * @param region
+   * @throws Exception
+   */
   private void processRegionClose(HRegionInfo region) {
     synchronized (master.regionManager) {
       if (region.isRootRegion()) {