HBASE-1105 Remove duplicated code in HCM, add javadoc to RegionState, etc.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/hbase/trunk@730494 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -201,6 +201,7 @@ Release 0.19.0 - Unreleased
                operations on individual regions
    HBASE-1062  Compactions at (re)start on a large table can overwhelm DFS
    HBASE-1102  boolean HTable.exists()
+   HBASE-1105  Remove duplicated code in HCM, add javadoc to RegionState, etc.
 
   NEW FEATURES
    HBASE-875   Use MurmurHash instead of JenkinsHash [in bloomfilters]

bin/HBase.rb

@@ -418,7 +418,7 @@ module HBase
       s = @table.getScanner(cs)
       count = 0
       i = s.iterator()
-      @formatter.header("Count may take a long time to complete!")
+      @formatter.header()
       while i.hasNext()
         r = i.next()
         count += 1

src/java/org/apache/hadoop/hbase/client/HConnectionManager.java

@@ -137,8 +137,8 @@ public class HConnectionManager implements HConstants {
     private volatile HRegionLocation rootRegionLocation; 
     
     private final Map<Integer, SoftValueSortedMap<byte [], HRegionLocation>> 
-      cachedRegionLocations = Collections.synchronizedMap(
+      cachedRegionLocations =
-         new HashMap<Integer, SoftValueSortedMap<byte [], HRegionLocation>>());
+        new HashMap<Integer, SoftValueSortedMap<byte [], HRegionLocation>>();
     
     /** 
      * constructor
@@ -454,7 +454,6 @@ public class HConnectionManager implements HConstants {
           // This block guards against two threads trying to load the meta 
           // region at the same time. The first will load the meta region and
           // the second will use the value that the first one found.
-
           return locateRegionInMeta(ROOT_TABLE_NAME, tableName, row, useCache);
         }
       } else {
@@ -464,7 +463,7 @@ public class HConnectionManager implements HConstants {
       }
     }
 
-    /**
+    /*
       * Search one of the meta tables (-ROOT- or .META.) for the HRegionLocation
       * info that contains the table and row we're seeking.
       */
@@ -472,9 +471,8 @@ public class HConnectionManager implements HConstants {
       final byte [] tableName, final byte [] row, boolean useCache)
     throws IOException{
       HRegionLocation location = null;
-      // if we're supposed to be using the cache, then check it for a possible
+      // If supposed to be using the cache, then check it for a possible hit.
-      // hit. otherwise, delete any existing cached location so it won't 
+      // Otherwise, delete any existing cached location so it won't interfere.
-      // interfere.
       if (useCache) {
         location = getCachedLocation(tableName, row);
         if (location != null) {
@@ -505,28 +503,23 @@ public class HConnectionManager implements HConstants {
           RowResult regionInfoRow = server.getClosestRowBefore(
             metaLocation.getRegionInfo().getRegionName(), metaKey,
             HConstants.COLUMN_FAMILY);
-
           if (regionInfoRow == null) {
             throw new TableNotFoundException(Bytes.toString(tableName));
           }
 
           Cell value = regionInfoRow.get(COL_REGIONINFO);
-
           if (value == null || value.getValue().length == 0) {
             throw new IOException("HRegionInfo was null or empty in " + 
               Bytes.toString(parentTable));
           }
-
           // convert the row result into the HRegionLocation we need!
           HRegionInfo regionInfo = (HRegionInfo) Writables.getWritable(
               value.getValue(), new HRegionInfo());
-
           // possible we got a region of a different table...
           if (!Bytes.equals(regionInfo.getTableDesc().getName(), tableName)) {
             throw new TableNotFoundException(
               "Table '" + Bytes.toString(tableName) + "' was not found.");
           }
-
           if (regionInfo.isOffline()) {
             throw new RegionOfflineException("region offline: " + 
               regionInfo.getRegionNameAsString());
@@ -534,7 +527,6 @@ public class HConnectionManager implements HConstants {
           
           String serverAddress = 
             Writables.cellToString(regionInfoRow.get(COL_SERVER));
-        
           if (serverAddress.equals("")) { 
             throw new NoServerForRegionException("No server address listed " +
               "in " + Bytes.toString(parentTable) + " for region " +
@@ -544,9 +536,7 @@ public class HConnectionManager implements HConstants {
           // instantiate the location
           location = new HRegionLocation(regionInfo,
               new HServerAddress(serverAddress));
-      
           cacheLocation(tableName, location);
-
           return location;
         } catch (TableNotFoundException e) {
           // if we got this error, probably means the table just plain doesn't
@@ -591,17 +581,8 @@ public class HConnectionManager implements HConstants {
      */
     private HRegionLocation getCachedLocation(final byte [] tableName,
         final byte [] row) {
-      // find the map of cached locations for this table
-      Integer key = Bytes.mapKey(tableName);
       SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
-        cachedRegionLocations.get(key);
+        getTableLocations(tableName);
-
-      // if tableLocations for this table isn't built yet, make one
-      if (tableLocations == null) {
-        tableLocations = new SoftValueSortedMap<byte [],
-          HRegionLocation>(Bytes.BYTES_COMPARATOR);
-        cachedRegionLocations.put(key, tableLocations);
-      }
 
       // start to examine the cache. we can only do cache actions
       // if there's something in the cache for this table.
@@ -612,9 +593,9 @@ public class HConnectionManager implements HConstants {
       HRegionLocation rl = tableLocations.get(row);
       if (rl != null) {
         if (LOG.isDebugEnabled()) {
-          LOG.debug("Cache hit in table locations for row <" +
+          LOG.debug("Cache hit for row <" +
             Bytes.toString(row) +
-            "> and tableName " + Bytes.toString(tableName) +
+            "> in tableName " + Bytes.toString(tableName) +
             ": location server " + rl.getServerAddress() +
             ", location region name " +
             rl.getRegionInfo().getRegionNameAsString());
@@ -657,23 +638,14 @@ public class HConnectionManager implements HConstants {
       return null;
     }
 
-    /**
+    /*
      * Delete a cached location, if it satisfies the table name and row
      * requirements.
      */
     private void deleteCachedLocation(final byte [] tableName,
         final byte [] row) {
-      // find the map of cached locations for this table
-      Integer key = Bytes.mapKey(tableName);
       SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
-        cachedRegionLocations.get(key);
+        getTableLocations(tableName);
-
-      // if tableLocations for this table isn't built yet, make one
-      if (tableLocations == null) {
-        tableLocations = new SoftValueSortedMap<byte [],
-          HRegionLocation>(Bytes.BYTES_COMPARATOR);
-        cachedRegionLocations.put(key, tableLocations);
-      }
 
       // start to examine the cache. we can only do cache actions
       // if there's something in the cache for this table.
@@ -689,7 +661,6 @@ public class HConnectionManager implements HConstants {
         if (!matchingRegions.isEmpty()) {
           HRegionLocation possibleRegion =
             matchingRegions.get(matchingRegions.lastKey());
-          
           byte [] endKey = possibleRegion.getRegionInfo().getEndKey();
 
           // by nature of the map, we know that the start key has to be < 
@@ -701,33 +672,43 @@ public class HConnectionManager implements HConstants {
               tableLocations.remove(matchingRegions.lastKey());
             if (rl != null && LOG.isDebugEnabled()) {
               LOG.debug("Removed " + rl.getRegionInfo().getRegionNameAsString() +
-                " from cache because of " + Bytes.toString(row));
+                " for tableName=" + Bytes.toString(tableName) + " from cache " +
+                "because of " + Bytes.toString(row));
             }
           }
         }
       }
     }
     
-    /**
+    /*
+     * @param tableName
+     * @return Map of cached locations for passed <code>tableName</code>
+     */
+    private SoftValueSortedMap<byte [], HRegionLocation> getTableLocations(
+        final byte [] tableName) {
+      // find the map of cached locations for this table
+      Integer key = Bytes.mapKey(tableName);
+      SoftValueSortedMap<byte [], HRegionLocation> result = null;
+      synchronized (this.cachedRegionLocations) {
+        result = this.cachedRegionLocations.get(key);
+        // if tableLocations for this table isn't built yet, make one
+        if (result == null) {
+          result = new SoftValueSortedMap<byte [], HRegionLocation>(
+              Bytes.BYTES_COMPARATOR);
+          this.cachedRegionLocations.put(key, result);
+        }
+      }
+      return result;
+    }
+
+    /*
      * Put a newly discovered HRegionLocation into the cache.
      */
     private void cacheLocation(final byte [] tableName,
         final HRegionLocation location) {
       byte [] startKey = location.getRegionInfo().getStartKey();
-      
-      // find the map of cached locations for this table
-      Integer key = Bytes.mapKey(tableName);
       SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
-        cachedRegionLocations.get(key);
+        getTableLocations(tableName);
-
-      // if tableLocations for this table isn't built yet, make one
-      if (tableLocations == null) {
-        tableLocations = new SoftValueSortedMap<byte [],
-          HRegionLocation>(Bytes.BYTES_COMPARATOR);
-        cachedRegionLocations.put(key, tableLocations);
-      }
-      
-      // save the HRegionLocation under the startKey
       tableLocations.put(startKey, location);
     }
     
@@ -932,8 +913,9 @@ public class HConnectionManager implements HConstants {
             }
             long sleepTime = getPauseTime(tries);
             if (LOG.isDebugEnabled()) {
-              LOG.debug("Reloading table servers because region " +
+              LOG.debug("Reloading region " + Bytes.toString(currentRegion) +
-                "server didn't accept updates; tries=" + tries +
+                " location because regionserver didn't accept updates; " +
+                "tries=" + tries +
                 " of max=" + this.numRetries + ", waiting=" + sleepTime + "ms");
             }
             try {
@@ -946,7 +928,6 @@ public class HConnectionManager implements HConstants {
             retryOnlyOne = true;
             region = getRegionLocation(tableName, list.get(i + 1).getRow(),
                 true).getRegionInfo().getRegionName();
-            
           }
           else {
             retryOnlyOne = false;

src/java/org/apache/hadoop/hbase/master/RegionManager.java

@@ -90,6 +90,7 @@ class RegionManager implements HConstants {
    * 
    * Note: Needs to be SortedMap so we can specify a comparator
    * 
+   * @see RegionState inner-class below
    */
   private final SortedMap<byte[], RegionState> regionsInTransition =
     Collections.synchronizedSortedMap(
@@ -1013,38 +1014,38 @@ class RegionManager implements HConstants {
   }
   
   /*
-   * State of a Region.
+   * State of a Region as it transitions from closed to open, etc.  See
-   * Used while regions are making transitions from unassigned to assigned to
+   * note on regionsInTransition data member above for listing of state
-   * opened, etc.
+   * transitions.
    */
   private static class RegionState implements Comparable<RegionState> {
-    private final byte [] regionName;
+    private final HRegionInfo regionInfo;
-    private HRegionInfo regionInfo = null;
     private volatile boolean unassigned = false;
     private volatile boolean assigned = false;
     private volatile boolean pending = false;
     private volatile boolean closing = false;
     private volatile boolean closed = false;
     private volatile boolean offlined = false;
+    
+    /* Set when region is assigned.
+     */
     private String serverName = null;
 
-    RegionState(byte [] regionName) {
-      this.regionName = regionName;
-    }
-    
     RegionState(HRegionInfo info) {
-      this.regionName = info.getRegionName();
       this.regionInfo = info;
     }
     
     byte [] getRegionName() {
-      return regionName;
+      return this.regionInfo.getRegionName();
     }
 
     synchronized HRegionInfo getRegionInfo() {
       return this.regionInfo;
     }
 
+    /*
+     * @return Server this region was assigned to
+     */
     synchronized String getServerName() {
       return this.serverName;
     }
@@ -1070,7 +1071,10 @@ class RegionManager implements HConstants {
       return assigned;
     }
 
-    synchronized void setAssigned(String serverName) {
+    /*
+     * @param serverName Server region was assigned to.
+     */
+    synchronized void setAssigned(final String serverName) {
       if (!this.unassigned) {
         throw new IllegalStateException(
             "Cannot assign a region that is not currently unassigned. " +
@@ -1132,7 +1136,7 @@ class RegionManager implements HConstants {
 
     @Override
     public synchronized String toString() {
-      return "name=" + Bytes.toString(this.regionName) +
+      return "name=" + Bytes.toString(getRegionName()) +
           ", isUnassigned=" + this.unassigned + ", isAssigned=" +
           this.assigned + ", isPending=" + this.pending + ", isClosing=" +
           this.closing + ", isClosed=" + this.closed + ", isOfflined=" +
@@ -1146,7 +1150,7 @@ class RegionManager implements HConstants {
     
     @Override
     public int hashCode() {
-      return Bytes.toString(regionName).hashCode();
+      return Bytes.toString(getRegionName()).hashCode();
     }
     
     @Override
@@ -1154,7 +1158,7 @@ class RegionManager implements HConstants {
       if (o == null) {
         return 1;
       }
-      return Bytes.compareTo(this.regionName, o.getRegionName());
+      return Bytes.compareTo(getRegionName(), o.getRegionName());
     }
   }
 }