hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r653642 - in /hadoop/hbase/branches/0.1: CHANGES.txt src/java/org/apache/hadoop/hbase/HMaster.java src/java/org/apache/hadoop/hbase/HStore.java src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
Date Mon, 05 May 2008 23:56:32 GMT
Author: jimk
Date: Mon May  5 16:56:31 2008
New Revision: 653642

URL: http://svn.apache.org/viewvc?rev=653642&view=rev
Log:
HBASE-478   offlining of table does not run reliably

Modified:
    hadoop/hbase/branches/0.1/CHANGES.txt
    hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java
    hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
    hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java

Modified: hadoop/hbase/branches/0.1/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/CHANGES.txt?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/CHANGES.txt (original)
+++ hadoop/hbase/branches/0.1/CHANGES.txt Mon May  5 16:56:31 2008
@@ -30,7 +30,8 @@
    HBASE-609   Master doesn't see regionserver edits because of clock skew
    HBASE-607   MultiRegionTable.makeMultiRegionTable is not deterministic enough
                for regression tests
-
+   HBASE-478   offlining of table does not run reliably
+   
   IMPROVEMENTS
    HBASE-559   MR example job to count table rows
    HBASE-578   Upgrade branch to 0.16.3 hadoop.

Modified: hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java (original)
+++ hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HMaster.java Mon May  5 16:56:31
2008
@@ -430,10 +430,10 @@
     throws IOException {
       // Skip region - if ...
       if (info.isOffline()                                 // offline
-          || killedRegions.contains(info.getRegionName())  // queued for offline
-          || regionsToDelete.contains(info.getRegionName())) { // queued for delete
+          || killedRegions.contains(info.getRegionName())) {  // queued for offline
 
         unassignedRegions.remove(info);
+        pendingRegions.remove(info);
         return;
       }
       HServerInfo storedInfo = null;
@@ -846,13 +846,6 @@
   volatile Set<Text> killedRegions =
     Collections.synchronizedSet(new HashSet<Text>());
 
-  /**
-   * 'regionsToDelete' contains regions that need to be deleted, but cannot be
-   * until the region server closes it
-   */
-  volatile Set<Text> regionsToDelete =
-    Collections.synchronizedSet(new HashSet<Text>());
-
   /** Set of tables currently in creation. */
   private volatile Set<Text> tableInCreation = 
     Collections.synchronizedSet(new HashSet<Text>());
@@ -1414,8 +1407,9 @@
                   } else if (info.isMetaTable()) {
                     onlineMetaRegions.remove(info.getStartKey());
                   }
-
-                  this.unassignedRegions.put(info, ZERO_L);
+                  if (!killedRegions.remove(info.getRegionName())) {
+                    this.unassignedRegions.put(info, ZERO_L);
+                  }
                 }
               }
             }
@@ -1678,14 +1672,9 @@
           unassignRootRegion();
         } else {
           boolean reassignRegion = !region.isOffline();
-          boolean deleteRegion = false;
           if (killedRegions.remove(region.getRegionName())) {
             reassignRegion = false;
           }
-          if (regionsToDelete.remove(region.getRegionName())) {
-            reassignRegion = false;
-            deleteRegion = true;
-          }
           if (region.isMetaTable()) {
             // Region is part of the meta table. Remove it from onlineMetaRegions
             onlineMetaRegions.remove(region.getStartKey());
@@ -1696,8 +1685,7 @@
           //       reassigned before the close is processed.
           unassignedRegions.remove(region);
           try {
-            toDoQueue.put(new ProcessRegionClose(region, reassignRegion,
-                deleteRegion));
+            toDoQueue.put(new ProcessRegionClose(region, reassignRegion));
 
           } catch (InterruptedException e) {
             throw new RuntimeException(
@@ -2008,13 +1996,11 @@
     private boolean rootRescanned;
 
     private class ToDoEntry {
-      boolean deleteRegion;
       boolean regionOffline;
       Text row;
       HRegionInfo info;
 
       ToDoEntry(Text row, HRegionInfo info) {
-        this.deleteRegion = false;
         this.regionOffline = false;
         this.row = row;
         this.info = info;
@@ -2106,35 +2092,26 @@
           ToDoEntry todo = new ToDoEntry(row, info);
           toDoList.add(todo);
 
-          if (killList.containsKey(deadServerName)) {
-            HashMap<Text, HRegionInfo> regionsToKill =
-              new HashMap<Text, HRegionInfo>();
-            synchronized (killList) {
+          synchronized (killList) {
+            if (killList.containsKey(deadServerName)) {
+              HashMap<Text, HRegionInfo> regionsToKill =
+                new HashMap<Text, HRegionInfo>();
+
               regionsToKill.putAll(killList.get(deadServerName));
-            }
 
-            if (regionsToKill.containsKey(info.getRegionName())) {
-              regionsToKill.remove(info.getRegionName());
-              killList.put(deadServerName, regionsToKill);
-              unassignedRegions.remove(info);
-              synchronized (regionsToDelete) {
-                if (regionsToDelete.contains(info.getRegionName())) {
-                  // Delete this region
-                  regionsToDelete.remove(info.getRegionName());
-                  todo.deleteRegion = true;
-                } else {
-                  // Mark region offline
-                  todo.regionOffline = true;
-                }
+              if (regionsToKill.containsKey(info.getRegionName())) {
+                regionsToKill.remove(info.getRegionName());
+                killList.put(deadServerName, regionsToKill);
+                unassignedRegions.remove(info);
+                // Mark region offline
+                todo.regionOffline = true;
               }
-            }
-
-          } else {
-            // Get region reassigned
-            regions.add(info);
 
+            } else {
+              // Get region reassigned
+              regions.add(info);
+            }
             // If it was pending, remove.
-            // Otherwise will obstruct its getting reassigned.
             pendingRegions.remove(info.getRegionName());
           }
         }
@@ -2160,9 +2137,7 @@
       
       // Update server in root/meta entries
       for (ToDoEntry e: toDoList) {
-        if (e.deleteRegion) {
-          HRegion.removeRegionFromMETA(server, regionName, e.row);
-        } else if (e.regionOffline) {
+        if (e.regionOffline) {
           HRegion.offlineRegionInMETA(server, regionName, e.info);
         }
       }
@@ -2282,6 +2257,7 @@
                   Thread.currentThread().getName());
             }
           }
+          killList.remove(deadServerName);
           deadServers.remove(deadServerName);
           break;
 
@@ -2373,26 +2349,21 @@
    */
   private class ProcessRegionClose extends ProcessRegionStatusChange {
     private boolean reassignRegion;
-    private boolean deleteRegion;
 
     /**
      * @param regionInfo
      * @param reassignRegion
-     * @param deleteRegion
      */
-    public ProcessRegionClose(HRegionInfo regionInfo, boolean reassignRegion,
-        boolean deleteRegion) {
-
+    public ProcessRegionClose(HRegionInfo regionInfo, boolean reassignRegion) {
       super(regionInfo);
       this.reassignRegion = reassignRegion;
-      this.deleteRegion = deleteRegion;
     }
 
     /** {@inheritDoc} */
     @Override
     public String toString() {
       return "ProcessRegionClose of " + this.regionInfo.getRegionName() +
-        ", " + this.reassignRegion + ", " + this.deleteRegion;
+        ", " + this.reassignRegion;
     }
 
     @Override
@@ -2414,10 +2385,7 @@
         }
 
         try {
-          if (deleteRegion) {
-            HRegion.removeRegionFromMETA(getMetaServer(), metaRegionName,
-              regionInfo.getRegionName());
-          } else if (!this.reassignRegion) {
+          if (!this.reassignRegion) {
             HRegion.offlineRegionInMETA(getMetaServer(), metaRegionName,
               regionInfo);
           }
@@ -2435,15 +2403,6 @@
         LOG.info("reassign region: " + regionInfo.getRegionName());
 
         unassignedRegions.put(regionInfo, ZERO_L);
-
-      } else if (deleteRegion) {
-        try {
-          HRegion.deleteRegion(fs, rootdir, regionInfo);
-        } catch (IOException e) {
-          e = RemoteExceptionHandler.checkIOException(e);
-          LOG.error("failed delete region " + regionInfo.getRegionName(), e);
-          throw e;
-        }
       }
       return true;
     }
@@ -2775,8 +2734,8 @@
                   HRegionInfo info = getHRegionInfo(row, map);
                   if (info == null) {
                     emptyRows.add(row);
-                    throw new IOException(COL_REGIONINFO + " not found on " +
-                      row);
+                    LOG.error(COL_REGIONINFO + " not found on " + row);
+                    continue;
                   }
                   String serverName = Writables.bytesToString(map.get(COL_SERVER));
                   long startCode = Writables.bytesToLong(map.get(COL_STARTCODE));
@@ -2812,7 +2771,7 @@
               }
               
               if (!tableExists) {
-                throw new IOException(tableName + " does not exist");
+                throw new TableNotFoundException(tableName + " does not exist");
               }
 
               postProcessMeta(m, server);
@@ -2822,6 +2781,11 @@
           } // synchronized(metaScannerLock)
 
         } catch (IOException e) {
+          if (e instanceof TableNotFoundException ||
+              e instanceof TableNotDisabledException ||
+              e instanceof InvalidColumnNameException) {
+            throw e;
+          }
           if (tries == numRetries - 1) {
             // No retries left
             checkFileSystem();
@@ -2894,8 +2858,8 @@
       for (HRegionInfo i: unservedRegions) {
         if (i.isOffline() && i.isSplit()) {
           if (LOG.isDebugEnabled()) {
-            LOG.debug("Skipping region " + i.toString() + " because it is " +
-                "offline because it has been split");
+            LOG.debug("Skipping region " + i.toString() +
+                " because it is offline because it has been split");
           }
           continue;
         }
@@ -2917,8 +2881,11 @@
         }
 
         if (online) {                         // Bring offline regions on-line
-          if (!unassignedRegions.containsKey(i)) {
-            unassignedRegions.put(i, ZERO_L);
+          killedRegions.remove(i.getRegionName());
+          synchronized (unassignedRegions) {
+            if (!unassignedRegions.containsKey(i)) {
+              unassignedRegions.put(i, ZERO_L);
+            }
           }
 
         } else {                              // Prevent region from getting assigned.
@@ -2943,12 +2910,6 @@
         HashMap<Text, HRegionInfo> localKillList =
           new HashMap<Text, HRegionInfo>();
         
-        synchronized (killList) {
-          HashMap<Text, HRegionInfo> killedRegions = killList.get(serverName);
-          if (killedRegions != null) {
-            localKillList.putAll(killedRegions);
-          }
-        }
         for (HRegionInfo i: e.getValue()) {
           if (LOG.isDebugEnabled()) {
             LOG.debug("adding region " + i.getRegionName() +
@@ -2956,12 +2917,18 @@
           }
           localKillList.put(i.getRegionName(), i);
         }
-        if (localKillList.size() > 0) {
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("inserted local kill list into kill list for server " +
-                serverName);
+        synchronized (killList) {
+          HashMap<Text, HRegionInfo> killedRegions = killList.get(serverName);
+          if (killedRegions != null) {
+            localKillList.putAll(killedRegions);
+          }
+          if (localKillList.size() > 0) {
+            if (LOG.isDebugEnabled()) {
+              LOG.debug("inserted local kill list into kill list for server " +
+                  serverName);
+            }
+            killList.put(serverName, localKillList);
           }
-          killList.put(serverName, localKillList);
         }
       }
       servedRegions.clear();
@@ -2976,33 +2943,44 @@
   }
 
   /** 
-   * Instantiated to delete a table
-   * Note that it extends ChangeTableState, which takes care of disabling
-   * the table.
+   * Instantiated to delete a table. Table must be disabled first
    */
-  private class TableDelete extends ChangeTableState {
+  private class TableDelete extends TableOperation {
 
     TableDelete(Text tableName) throws IOException {
-      super(tableName, false);
+      super(tableName);
     }
 
     @Override
-    protected void postProcessMeta(MetaRegion m, HRegionInterface server)
-      throws IOException {
-
-      // For regions that are being served, mark them for deletion      
+    protected void processScanItem(
+        @SuppressWarnings("unused") String serverName,
+        @SuppressWarnings("unused") long startCode,
+        final HRegionInfo info) throws IOException {
       
-      for (HashSet<HRegionInfo> s: servedRegions.values()) {
-        for (HRegionInfo i: s) {
-          regionsToDelete.add(i.getRegionName());
-        }
+      if (isEnabled(info)) {
+        throw new TableNotDisabledException(tableName.toString());
       }
+    }
 
-      // Unserved regions we can delete now
-      
+    @Override
+    protected void postProcessMeta(MetaRegion m, HRegionInterface server)
+    throws IOException {
       for (HRegionInfo i: unservedRegions) {
+        // Update meta table
+        
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("updating columns in row: " + i.getRegionName());
+        }
+
+        BatchUpdate b = new BatchUpdate(rand.nextLong());
+        long lockid = b.startUpdate(i.getRegionName());
+        updateRegionInfo(lockid, b, i);
+        server.batchUpdate(m.getRegionName(), b);
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("updated columns in row: " + i.getRegionName());
+        }
+
         // Delete the region
-      
         try {
           HRegion.deleteRegion(fs, rootdir, i);
         
@@ -3011,11 +2989,9 @@
             RemoteExceptionHandler.checkIOException(e));
         }
       }
-      super.postProcessMeta(m, server);
     }
 
-    @Override
-    protected void updateRegionInfo(BatchUpdate b,
+    private void updateRegionInfo(long lockid, BatchUpdate b,
         @SuppressWarnings("unused") HRegionInfo info) {
       for (int i = 0; i < ALL_META_COLUMNS.length; i++) {
         // Be sure to clean all cells
@@ -3131,9 +3107,8 @@
         if (families.get(columnName) != null){
           families.put(columnName, descriptor);
           updateRegionInfo(server, m.getRegionName(), i);          
-        }
-        else{ // otherwise, we have an error.
-          throw new IOException("Column family '" + columnName + 
+        } else{ // otherwise, we have an error.
+          throw new InvalidColumnNameException("Column family '" + columnName + 
             "' doesn't exist, so cannot be modified.");
         }
       }

Modified: hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java (original)
+++ hadoop/hbase/branches/0.1/src/java/org/apache/hadoop/hbase/HStore.java Mon May  5 16:56:31
2008
@@ -356,7 +356,8 @@
       // if there are items in the tail map, there's either a direct match to
       // the search key, or a range of values between the first candidate key
       // and the ultimate search key (or the end of the cache)
-      if (!tailMap.isEmpty() && tailMap.firstKey().getRow().compareTo(key) <=
0) {
+      if (!tailMap.isEmpty() &&
+          tailMap.firstKey().getRow().compareTo(key) <= 0) {
         key_iterator = tailMap.keySet().iterator();
 
         // keep looking at cells as long as they are no greater than the 
@@ -519,8 +520,9 @@
      * equal or older timestamp.  If no keys, returns an empty List. Does not
      * return null.
      */
-    private List<HStoreKey> internalGetKeys(final SortedMap<HStoreKey, byte []>
map,
-        final HStoreKey origin, final int versions) {
+    private List<HStoreKey> internalGetKeys(
+        final SortedMap<HStoreKey, byte []> map, final HStoreKey origin,
+        final int versions) {
 
       List<HStoreKey> result = new ArrayList<HStoreKey>();
       SortedMap<HStoreKey, byte []> tailMap = map.tailMap(origin);
@@ -609,6 +611,8 @@
         }
       }
 
+      /** {@inheritDoc} */
+      @Override
       public boolean next(HStoreKey key, SortedMap<Text, byte []> results)
       throws IOException {
          if (this.scannerClosed) {
@@ -632,6 +636,9 @@
          key.setRow(this.currentRow);
          key.setVersion(this.timestamp);
          getFull(key, deletes, rowResults);
+         for (Text column: deletes.keySet()) {
+           rowResults.put(column, HLogEdit.deleteBytes.get());
+         }
          for (Map.Entry<Text, byte[]> e: rowResults.entrySet()) {
            Text column = e.getKey();
            byte [] c = e.getValue();
@@ -651,7 +658,9 @@
        }
        return results.size() > 0;
      }
-     public void close() {
+      
+    /** {@inheritDoc} */
+    public void close() {
        if (!scannerClosed) {
          scannerClosed = true;
        }
@@ -2361,9 +2370,11 @@
     // Values that correspond to those keys
     private byte [][] vals;
 
+    @SuppressWarnings("hiding")
     private MapFile.Reader[] readers;
 
     // Used around replacement of Readers if they change while we're scanning.
+    @SuppressWarnings("hiding")
     private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
     
     StoreFileScanner(long timestamp, Text[] targetCols, Text firstRow)
@@ -2531,17 +2542,21 @@
        this.ts = t;
      }
 
+     /** @return the row */
      public Text getRow() {
        return this.row;
      }
 
+     /** @return the timestamp */
      public long getTimestamp() {
        return this.ts;
      }
    }
    
    // Implementation of ChangedReadersObserver
-   public void updateReaders() throws IOException {
+   
+   /** {@inheritDoc} */
+  public void updateReaders() throws IOException {
      this.lock.writeLock().lock();
      try {
        // The keys are currently lined up at the next row to fetch.  Pass in
@@ -2565,8 +2580,8 @@
      */
     boolean findFirstRow(int i, Text firstRow) throws IOException {
       ImmutableBytesWritable ibw = new ImmutableBytesWritable();
-      HStoreKey firstKey
-        = (HStoreKey)readers[i].getClosest(new HStoreKey(firstRow), ibw);
+      HStoreKey firstKey =
+        (HStoreKey)readers[i].getClosest(new HStoreKey(firstRow), ibw);
       if (firstKey == null) {
         // Didn't find it. Close the scanner and return TRUE
         closeSubScanner(i);

Modified: hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java?rev=653642&r1=653641&r2=653642&view=diff
==============================================================================
--- hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java (original)
+++ hadoop/hbase/branches/0.1/src/test/org/apache/hadoop/hbase/TestHBaseCluster.java Mon May
 5 16:56:31 2008
@@ -65,7 +65,6 @@
     basic();
     scanner();
     listTables();
-    cleanup();
   }
 
   private static final int FIRST_ROW = 1;
@@ -199,11 +198,4 @@
     assertTrue(families.contains(new Text(CONTENTS)));
     assertTrue(families.contains(new Text(ANCHOR)));
   }
-  
-  private void cleanup() throws IOException {
-
-    // Delete the table we created
-
-    admin.deleteTable(desc.getName());
-  }
 }



Mime
View raw message