HBASE-23904 Procedure updating meta and Master shutdown are incompatible: CODE-BUG

Restore behavior from before HBASE-21789 (hbase-2.2.0) where we convert
all exceptions to IOEs, even RuntimeExceptions. Actual fix is this change (in case
obscured by doc and lambda simplification):

     } catch (Throwable e) {
-      Throwables.propagateIfPossible(e, IOException.class);
+      // Throw if an IOE else wrap in an IOE EVEN IF IT IS a RuntimeException (e.g.
+      // a RejectedExecutionException because the hosting exception is shutting down.
+      // This is old behavior worth reexamining. Procedures doing merge or split
+      // currently don't handle RuntimeExceptions coming up out of meta table edits.
+      // Would have to work on this at least. See HBASE-23904.
+      Throwables.throwIfInstanceOf(e, IOException.class);
This commit is contained in:
stack 2020-02-26 22:16:11 -08:00
parent 7df9490d60
commit 836e1a1caf
2 changed files with 54 additions and 33 deletions

View File

@ -232,8 +232,7 @@ public class MetaTableAccessor {
* @param connection connection we're using to access Meta
* @return An {@link Table} for <code>hbase:meta</code>
*/
public static Table getMetaHTable(final Connection connection)
throws IOException {
public static Table getMetaHTable(final Connection connection) throws IOException {
// We used to pass whole CatalogTracker in here, now we just pass in Connection
if (connection == null) {
throw new NullPointerException("No connection");
@ -1733,44 +1732,47 @@ public class MetaTableAccessor {
}
/**
* Performs an atomic multi-mutate operation against the given table.
* Performs an atomic multi-mutate operation against the given table. Used by the likes of
* merge and split as these want to make atomic mutations across multiple rows.
* @throws IOException even if we encounter a RuntimeException, we'll still wrap it in an IOE.
*/
private static void multiMutate(final Table table, byte[] row, final List<Mutation> mutations)
@VisibleForTesting
static void multiMutate(final Table table, byte[] row, final List<Mutation> mutations)
throws IOException {
debugLogMutations(mutations);
Batch.Call<MultiRowMutationService, MutateRowsResponse> callable =
new Batch.Call<MultiRowMutationService, MutateRowsResponse>() {
@Override
public MutateRowsResponse call(MultiRowMutationService instance) throws IOException {
MutateRowsRequest.Builder builder = MutateRowsRequest.newBuilder();
for (Mutation mutation : mutations) {
if (mutation instanceof Put) {
builder.addMutationRequest(
ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.PUT, mutation));
} else if (mutation instanceof Delete) {
builder.addMutationRequest(
ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.DELETE, mutation));
} else {
throw new DoNotRetryIOException(
"multi in MetaEditor doesn't support " + mutation.getClass().getName());
}
}
ServerRpcController controller = new ServerRpcController();
CoprocessorRpcUtils.BlockingRpcCallback<MutateRowsResponse> rpcCallback =
new CoprocessorRpcUtils.BlockingRpcCallback<>();
instance.mutateRows(controller, builder.build(), rpcCallback);
MutateRowsResponse resp = rpcCallback.get();
if (controller.failedOnException()) {
throw controller.getFailedOn();
}
return resp;
Batch.Call<MultiRowMutationService, MutateRowsResponse> callable = instance -> {
MutateRowsRequest.Builder builder = MutateRowsRequest.newBuilder();
for (Mutation mutation : mutations) {
if (mutation instanceof Put) {
builder.addMutationRequest(
ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.PUT, mutation));
} else if (mutation instanceof Delete) {
builder.addMutationRequest(
ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.DELETE, mutation));
} else {
throw new DoNotRetryIOException(
"multi in MetaEditor doesn't support " + mutation.getClass().getName());
}
};
}
ServerRpcController controller = new ServerRpcController();
CoprocessorRpcUtils.BlockingRpcCallback<MutateRowsResponse> rpcCallback =
new CoprocessorRpcUtils.BlockingRpcCallback<>();
instance.mutateRows(controller, builder.build(), rpcCallback);
MutateRowsResponse resp = rpcCallback.get();
if (controller.failedOnException()) {
throw controller.getFailedOn();
}
return resp;
};
try {
table.coprocessorService(MultiRowMutationService.class, row, row, callable);
} catch (Throwable e) {
Throwables.propagateIfPossible(e, IOException.class);
// Throw if an IOE else wrap in an IOE EVEN IF IT IS a RuntimeException (e.g.
// a RejectedExecutionException because the hosting exception is shutting down.
// This is old behavior worth reexamining. Procedures doing merge or split
// currently don't handle RuntimeExceptions coming up out of meta table edits.
// Would have to work on this at least. See HBASE-23904.
Throwables.throwIfInstanceOf(e, IOException.class);
throw new IOException(e);
}
}

View File

@ -23,6 +23,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.NavigableMap;
import org.apache.hadoop.hbase.client.ClusterConnection;
@ -30,7 +31,10 @@ import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.client.coprocessor.Batch;
import org.apache.hadoop.hbase.ipc.HBaseRpcController;
import org.apache.hadoop.hbase.protobuf.generated.MultiRowMutationProtos;
import org.apache.hadoop.hbase.testclassification.MiscTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
import org.apache.hadoop.hbase.util.Bytes;
@ -40,7 +44,9 @@ import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.internal.matchers.Any;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.slf4j.Logger;
@ -88,6 +94,19 @@ public class TestMetaTableAccessorNoCluster {
UTIL.shutdownMiniZKCluster();
}
/**
* Expect a IOE to come out of multiMutate, even if down in the depths we throw
* a RuntimeException. See HBASE-23904
*/
@Test (expected = IOException.class)
public void testMultiMutate() throws Throwable {
Table table = Mockito.mock(Table.class);
Mockito.when(table.coprocessorService(Mockito.any(),
Mockito.any(byte [].class), Mockito.any(byte [].class), Mockito.any(Batch.Call.class))).
thenThrow(new RuntimeException("FAIL TEST WITH RuntimeException!"));
MetaTableAccessor.multiMutate(table, HConstants.LAST_ROW, Collections.emptyList());
}
@Test
public void testGetHRegionInfo() throws IOException {
assertNull(MetaTableAccessor.getRegionInfo(new Result()));