HBASE-4225 NoSuchColumnFamilyException in multi doesn't say which family

is bad (Ramkrishna Vasudevan)


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

CHANGES.txt

@@ -209,6 +209,8 @@ Release 0.91.0 - Unreleased
    HBASE-4167  Potential leak of HTable instances when using HTablePool with
                PoolType.ThreadLocal (Karthick Sankarachary)
    HBASE-4239  HBASE-4012 introduced duplicate variable Bytes.LONG_BYTES
+   HBASE-4225  NoSuchColumnFamilyException in multi doesn't say which family
+               is bad (Ramkrishna Vasudevan)
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  

src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java

@@ -22,6 +22,7 @@ package org.apache.hadoop.hbase.client;
 
 import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HServerAddress;
+import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
 import org.apache.hadoop.hbase.util.Addressing;
 
 import java.util.Collection;
@@ -126,7 +127,12 @@ extends RetriesExhaustedException {
     Map<String, Integer> cls = new HashMap<String, Integer>();
     for (Throwable t : ths) {
       if (t == null) continue;
-      String name = t.getClass().getSimpleName();
+      String name = "";
+      if (t instanceof NoSuchColumnFamilyException) {
+        name = t.getMessage();
+      } else {
+        name = t.getClass().getSimpleName();
+      }
       Integer i = cls.get(name);
       if (i == null) {
         i = 0;

src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

@@ -1582,13 +1582,14 @@ public class HRegion implements HeapSize { // , Writable{
    */
   private static class BatchOperationInProgress<T> {
     T[] operations;
-    OperationStatusCode[] retCodes;
     int nextIndexToProcess = 0;
+    OperationStatus[] retCodeDetails;
 
     public BatchOperationInProgress(T[] operations) {
       this.operations = operations;
-      retCodes = new OperationStatusCode[operations.length];
+      this.retCodeDetails = new OperationStatus[operations.length];
-      Arrays.fill(retCodes, OperationStatusCode.NOT_RUN);
+      Arrays.fill(this.retCodeDetails, new OperationStatus(
+          OperationStatusCode.NOT_RUN));
     }
 
     public boolean isDone() {
@@ -1600,7 +1601,7 @@ public class HRegion implements HeapSize { // , Writable{
    * Perform a batch put with no pre-specified locks
    * @see HRegion#put(Pair[])
    */
-  public OperationStatusCode[] put(Put[] puts) throws IOException {
+  public OperationStatus[] put(Put[] puts) throws IOException {
     @SuppressWarnings("unchecked")
     Pair<Put, Integer> putsAndLocks[] = new Pair[puts.length];
 
@@ -1612,10 +1613,15 @@ public class HRegion implements HeapSize { // , Writable{
 
   /**
    * Perform a batch of puts.
-   * @param putsAndLocks the list of puts paired with their requested lock IDs.
+   * 
+   * @param putsAndLocks
+   *          the list of puts paired with their requested lock IDs.
+   * @return an array of OperationStatus which internally contains the
+   *         OperationStatusCode and the exceptionMessage if any.
    * @throws IOException
    */
-  public OperationStatusCode[] put(Pair<Put, Integer>[] putsAndLocks) throws IOException {
+  public OperationStatus[] put(
+      Pair<Put, Integer>[] putsAndLocks) throws IOException {
     BatchOperationInProgress<Pair<Put, Integer>> batchOp =
       new BatchOperationInProgress<Pair<Put,Integer>>(putsAndLocks);
 
@@ -1636,11 +1642,12 @@ public class HRegion implements HeapSize { // , Writable{
         requestFlush();
       }
     }
-    return batchOp.retCodes;
+    return batchOp.retCodeDetails;
   }
 
   @SuppressWarnings("unchecked")
-  private long doMiniBatchPut(BatchOperationInProgress<Pair<Put, Integer>> batchOp) throws IOException {
+  private long doMiniBatchPut(
+      BatchOperationInProgress<Pair<Put, Integer>> batchOp) throws IOException {
     /* Run coprocessor pre hook outside of locks to avoid deadlock */
     if (coprocessorHost != null) {
       List<Pair<Put, Integer>> ops =
@@ -1694,7 +1701,8 @@ public class HRegion implements HeapSize { // , Writable{
           checkFamilies(familyMap.keySet());
         } catch (NoSuchColumnFamilyException nscf) {
           LOG.warn("No such column family in batch put", nscf);
-          batchOp.retCodes[lastIndexExclusive] = OperationStatusCode.BAD_FAMILY;
+          batchOp.retCodeDetails[lastIndexExclusive] = new OperationStatus(
+              OperationStatusCode.BAD_FAMILY, nscf.getMessage());
           lastIndexExclusive++;
           continue;
         }
@@ -1724,7 +1732,8 @@ public class HRegion implements HeapSize { // , Writable{
       // ----------------------------------
       for (int i = firstIndex; i < lastIndexExclusive; i++) {
         // skip invalid
-        if (batchOp.retCodes[i] != OperationStatusCode.NOT_RUN) continue;
+        if (batchOp.retCodeDetails[i].getOperationStatusCode()
+            != OperationStatusCode.NOT_RUN) continue;
 
         updateKVTimestamps(
             familyMaps[i].values(),
@@ -1741,7 +1750,10 @@ public class HRegion implements HeapSize { // , Writable{
       WALEdit walEdit = new WALEdit();
       for (int i = firstIndex; i < lastIndexExclusive; i++) {
         // Skip puts that were determined to be invalid during preprocessing
-        if (batchOp.retCodes[i] != OperationStatusCode.NOT_RUN) continue;
+        if (batchOp.retCodeDetails[i].getOperationStatusCode()
+            != OperationStatusCode.NOT_RUN) {
+          continue;
+        }
 
         Put p = batchOp.operations[i].getFirst();
         if (!p.getWriteToWAL()) continue;
@@ -1757,9 +1769,13 @@ public class HRegion implements HeapSize { // , Writable{
       // ----------------------------------
       long addedSize = 0;
       for (int i = firstIndex; i < lastIndexExclusive; i++) {
-        if (batchOp.retCodes[i] != OperationStatusCode.NOT_RUN) continue;
+        if (batchOp.retCodeDetails[i].getOperationStatusCode()
+            != OperationStatusCode.NOT_RUN) {
+          continue;
+        }
         addedSize += applyFamilyMapToMemstore(familyMaps[i]);
-        batchOp.retCodes[i] = OperationStatusCode.SUCCESS;
+        batchOp.retCodeDetails[i] = new OperationStatus(
+            OperationStatusCode.SUCCESS);
       }
 
       // ------------------------------------
@@ -1768,7 +1784,10 @@ public class HRegion implements HeapSize { // , Writable{
       if (coprocessorHost != null) {
         for (int i = firstIndex; i < lastIndexExclusive; i++) {
           // only for successful puts
-          if (batchOp.retCodes[i] != OperationStatusCode.SUCCESS) continue;
+          if (batchOp.retCodeDetails[i].getOperationStatusCode()
+              != OperationStatusCode.SUCCESS) {
+            continue;
+          }
           Put p = batchOp.operations[i].getFirst();
           coprocessorHost.postPut(familyMaps[i], p.getWriteToWAL());
         }
@@ -1785,8 +1804,9 @@ public class HRegion implements HeapSize { // , Writable{
       }
       if (!success) {
         for (int i = firstIndex; i < lastIndexExclusive; i++) {
-          if (batchOp.retCodes[i] == OperationStatusCode.NOT_RUN) {
+          if (batchOp.retCodeDetails[i].getOperationStatusCode() == OperationStatusCode.NOT_RUN) {
-            batchOp.retCodes[i] = OperationStatusCode.FAILURE;
+            batchOp.retCodeDetails[i] = new OperationStatus(
+                OperationStatusCode.FAILURE);
           }
         }
       }

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -1741,9 +1741,9 @@ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler,
       }
 
       this.requestCount.addAndGet(puts.size());
-      OperationStatusCode[] codes = region.put(putsWithLocks);
+      OperationStatus codes[] = region.put(putsWithLocks);
       for (i = 0; i < codes.length; i++) {
-        if (codes[i] != OperationStatusCode.SUCCESS) {
+        if (codes[i].getOperationStatusCode() != OperationStatusCode.SUCCESS) {
           return i;
         }
       }
@@ -2833,19 +2833,20 @@ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler,
 
           this.requestCount.addAndGet(puts.size());
 
-          OperationStatusCode[] codes =
+          OperationStatus[] codes =
               region.put(putsWithLocks.toArray(new Pair[]{}));
 
           for( int i = 0 ; i < codes.length ; i++) {
-            OperationStatusCode code = codes[i];
+            OperationStatus code = codes[i];
 
             Action<R> theAction = puts.get(i);
             Object result = null;
 
-            if (code == OperationStatusCode.SUCCESS) {
+            if (code.getOperationStatusCode() == OperationStatusCode.SUCCESS) {
               result = new Result();
-            } else if (code == OperationStatusCode.BAD_FAMILY) {
+            } else if (code.getOperationStatusCode()
-              result = new NoSuchColumnFamilyException();
+                == OperationStatusCode.BAD_FAMILY) {
+              result = new NoSuchColumnFamilyException(code.getExceptionMsg());
             }
             // FAILURE && NOT_RUN becomes null, aka: need to run again.
 

src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java

@@ -369,10 +369,11 @@ public class TestHRegion extends HBaseTestCase {
       puts[i].add(cf, qual, val);
     }
 
-    OperationStatusCode[] codes = this.region.put(puts);
+    OperationStatus[] codes = this.region.put(puts);
     assertEquals(10, codes.length);
     for (int i = 0; i < 10; i++) {
-      assertEquals(OperationStatusCode.SUCCESS, codes[i]);
+      assertEquals(OperationStatusCode.SUCCESS, codes[i]
+          .getOperationStatusCode());
     }
     assertEquals(1, HLog.getSyncOps());
 
@@ -382,7 +383,7 @@ public class TestHRegion extends HBaseTestCase {
     assertEquals(10, codes.length);
     for (int i = 0; i < 10; i++) {
       assertEquals((i == 5) ? OperationStatusCode.BAD_FAMILY :
-        OperationStatusCode.SUCCESS, codes[i]);
+        OperationStatusCode.SUCCESS, codes[i].getOperationStatusCode());
     }
     assertEquals(1, HLog.getSyncOps());
 
@@ -391,8 +392,8 @@ public class TestHRegion extends HBaseTestCase {
 
     MultithreadedTestUtil.TestContext ctx =
       new MultithreadedTestUtil.TestContext(HBaseConfiguration.create());
-    final AtomicReference<OperationStatusCode[]> retFromThread =
+    final AtomicReference<OperationStatus[]> retFromThread =
-      new AtomicReference<OperationStatusCode[]>();
+      new AtomicReference<OperationStatus[]>();
     TestThread putter = new TestThread(ctx) {
       @Override
       public void doWork() throws IOException {
@@ -420,7 +421,7 @@ public class TestHRegion extends HBaseTestCase {
     codes = retFromThread.get();
     for (int i = 0; i < 10; i++) {
       assertEquals((i == 5) ? OperationStatusCode.BAD_FAMILY :
-        OperationStatusCode.SUCCESS, codes[i]);
+        OperationStatusCode.SUCCESS, codes[i].getOperationStatusCode());
     }
 
     LOG.info("Nexta, a batch put which uses an already-held lock");
@@ -437,7 +438,7 @@ public class TestHRegion extends HBaseTestCase {
     LOG.info("...performed put");
     for (int i = 0; i < 10; i++) {
       assertEquals((i == 5) ? OperationStatusCode.BAD_FAMILY :
-        OperationStatusCode.SUCCESS, codes[i]);
+        OperationStatusCode.SUCCESS, codes[i].getOperationStatusCode());
     }
     // Make sure we didn't do an extra batch
     assertEquals(1, HLog.getSyncOps());