From c2af155e3580a66d840534344f226c4a83bc2f33 Mon Sep 17 00:00:00 2001 From: Zhihong Yu Date: Tue, 23 Aug 2011 22:32:16 +0000 Subject: [PATCH] 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 | 2 + .../RetriesExhaustedWithDetailsException.java | 8 ++- .../hadoop/hbase/regionserver/HRegion.java | 52 +++++++++++++------ .../hbase/regionserver/HRegionServer.java | 15 +++--- .../hbase/regionserver/TestHRegion.java | 15 +++--- 5 files changed, 61 insertions(+), 31 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index f6d5c0ff631..8cf352706de 100644 --- a/CHANGES.txt +++ b/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) diff --git a/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java b/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java index 9d188896aaa..4997143053f 100644 --- a/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedWithDetailsException.java +++ b/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 cls = new HashMap(); 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; diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 9250fc19dc3..2390718a99a 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/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[] operations; - OperationStatusCode[] retCodes; int nextIndexToProcess = 0; + OperationStatus[] retCodeDetails; public BatchOperationInProgress(T[] operations) { this.operations = operations; - retCodes = new OperationStatusCode[operations.length]; - Arrays.fill(retCodes, OperationStatusCode.NOT_RUN); + this.retCodeDetails = new OperationStatus[operations.length]; + 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 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[] putsAndLocks) throws IOException { + public OperationStatus[] put( + Pair[] putsAndLocks) throws IOException { BatchOperationInProgress> batchOp = new BatchOperationInProgress>(putsAndLocks); @@ -1636,11 +1642,12 @@ public class HRegion implements HeapSize { // , Writable{ requestFlush(); } } - return batchOp.retCodes; + return batchOp.retCodeDetails; } @SuppressWarnings("unchecked") - private long doMiniBatchPut(BatchOperationInProgress> batchOp) throws IOException { + private long doMiniBatchPut( + BatchOperationInProgress> batchOp) throws IOException { /* Run coprocessor pre hook outside of locks to avoid deadlock */ if (coprocessorHost != null) { List> 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) { - batchOp.retCodes[i] = OperationStatusCode.FAILURE; + if (batchOp.retCodeDetails[i].getOperationStatusCode() == OperationStatusCode.NOT_RUN) { + batchOp.retCodeDetails[i] = new OperationStatus( + OperationStatusCode.FAILURE); } } } diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index a817c37377a..3efba7f7fca 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/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 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) { - result = new NoSuchColumnFamilyException(); + } else if (code.getOperationStatusCode() + == OperationStatusCode.BAD_FAMILY) { + result = new NoSuchColumnFamilyException(code.getExceptionMsg()); } // FAILURE && NOT_RUN becomes null, aka: need to run again. diff --git a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index 8e9c8927210..9dbdbbc4d0b 100644 --- a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/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 retFromThread = - new AtomicReference(); + final AtomicReference retFromThread = + new AtomicReference(); 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());