From a55f6ac11e48a746b6e31d8410f5fd1eb6a4c9b1 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Fri, 12 May 2017 15:50:16 -0700 Subject: [PATCH] HBASE-18043 Institute a hard limit for individual cell size that cannot be overridden by clients --- .../java/org/apache/hadoop/hbase/HConstants.java | 3 ++ hbase-common/src/main/resources/hbase-default.xml | 9 ++++++ .../apache/hadoop/hbase/regionserver/HRegion.java | 12 ++++++-- .../hadoop/hbase/regionserver/RSRpcServices.java | 35 ++++++++++++++++++++-- .../hadoop/hbase/client/TestFromClientSide.java | 32 ++++++++++++++++++++ 5 files changed, 87 insertions(+), 4 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java index 3c1b021ec4..1bd81fe547 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java @@ -1360,6 +1360,9 @@ public final class HConstants { public static final String DEFAULT_SNAPSHOT_RESTORE_FAILSAFE_NAME = "hbase-failsafe-{snapshot.name}-{restore.timestamp}"; + public static final String HBASE_MAX_CELL_SIZE_KEY = "hbase.server.keyvalue.maxsize"; + public static final int DEFAULT_MAX_CELL_SIZE = 10485760; + private HConstants() { // Can't be instantiated with this ctor. } diff --git a/hbase-common/src/main/resources/hbase-default.xml b/hbase-common/src/main/resources/hbase-default.xml index 40cf6755f8..a6e37ef61e 100644 --- a/hbase-common/src/main/resources/hbase-default.xml +++ b/hbase-common/src/main/resources/hbase-default.xml @@ -532,6 +532,15 @@ possible configurations would overwhelm and obscure the important. or less disables the check. + hbase.server.keyvalue.maxsize + 10485760 + Maximum allowed size of an individual cell, inclusive of value and all key + components. A value of 0 or less disables the check. + The default value is 10MB. + This is a safety setting to protect the server from OOM situations. + + + hbase.client.scanner.timeout.period 60000 Client scanner lease period in milliseconds. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 4836dc82a8..db7cdb66d2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -1,5 +1,5 @@ /** - * Licensed to the Apache Software Foundation (ASF) under one +hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information * regarding copyright ownership. The ASF licenses this file @@ -307,6 +307,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // purge timeout, when a RPC call will be terminated by the RPC engine. final long maxBusyWaitDuration; + // Max cell size. If nonzero, the maximum allowed size for any given cell + // in bytes + final long maxCellSize; + // negative number indicates infinite timeout static final long DEFAULT_ROW_PROCESSOR_TIMEOUT = 60 * 1000L; final ExecutorService rowProcessorExecutor = Executors.newCachedThreadPool(); @@ -801,6 +805,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi false : conf.getBoolean(HConstants.ENABLE_CLIENT_BACKPRESSURE, HConstants.DEFAULT_ENABLE_CLIENT_BACKPRESSURE); + + this.maxCellSize = conf.getLong(HConstants.HBASE_MAX_CELL_SIZE_KEY, + HConstants.DEFAULT_MAX_CELL_SIZE); + boolean unassignForFNFE = conf.getBoolean(HREGION_UNASSIGN_FOR_FNFE, DEFAULT_HREGION_UNASSIGN_FOR_FNFE); if (unassignForFNFE) { @@ -7613,7 +7621,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi ClassSize.OBJECT + ClassSize.ARRAY + 49 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT + - (14 * Bytes.SIZEOF_LONG) + + (15 * Bytes.SIZEOF_LONG) + 6 * Bytes.SIZEOF_BOOLEAN); // woefully out of date - currently missing: diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 95408b7f2b..09088cba6a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -525,7 +525,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } switch (type) { case PUT: - rm.add(ProtobufUtil.toPut(action.getMutation(), cellScanner)); + Put put = ProtobufUtil.toPut(action.getMutation(), cellScanner); + checkCellSizeLimit(region, put); + rm.add(put); break; case DELETE: rm.add(ProtobufUtil.toDelete(action.getMutation(), cellScanner)); @@ -577,7 +579,9 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } switch (type) { case PUT: - rm.add(ProtobufUtil.toPut(action.getMutation(), cellScanner)); + Put put = ProtobufUtil.toPut(action.getMutation(), cellScanner); + checkCellSizeLimit(region, put); + rm.add(put); break; case DELETE: rm.add(ProtobufUtil.toDelete(action.getMutation(), cellScanner)); @@ -611,6 +615,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, throws IOException { long before = EnvironmentEdgeManager.currentTime(); Append append = ProtobufUtil.toAppend(mutation, cellScanner); + checkCellSizeLimit(region, append); quota.addMutation(append); Result r = null; if (region.getCoprocessorHost() != null) { @@ -659,6 +664,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, throws IOException { long before = EnvironmentEdgeManager.currentTime(); Increment increment = ProtobufUtil.toIncrement(mutation, cells); + checkCellSizeLimit(region, increment); quota.addMutation(increment); Result r = null; if (region.getCoprocessorHost() != null) { @@ -867,6 +873,26 @@ public class RSRpcServices implements HBaseRPCErrorHandler, return cellsToReturn; } + private void checkCellSizeLimit(final Region region, final Mutation m) throws IOException { + if (!(region instanceof HRegion)) { + return; + } + HRegion r = (HRegion)region; + if (r.maxCellSize > 0) { + CellScanner cells = m.cellScanner(); + while (cells.advance()) { + int size = CellUtil.estimatedSerializedSizeOf(cells.current()); + if (size > r.maxCellSize) { + String msg = "Cell with size " + size + " exceeds limit of " + r.maxCellSize + " bytes"; + if (LOG.isDebugEnabled()) { + LOG.debug(msg); + } + throw new DoNotRetryIOException(msg); + } + } + } + } + /** * Execute a list of Put/Delete mutations. * @@ -902,15 +928,18 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } mutationActionMap.put(mutation, action); mArray[i++] = mutation; + checkCellSizeLimit(region, mutation); quota.addMutation(mutation); } if (!region.getRegionInfo().isMetaTable()) { regionServer.cacheFlusher.reclaimMemStoreMemory(); } + // HBASE-17924 // sort to improve lock efficiency Arrays.sort(mArray); + OperationStatus[] codes = region.batchMutate(mArray, HConstants.NO_NONCE, HConstants.NO_NONCE); for (i = 0; i < codes.length; i++) { @@ -2604,6 +2633,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, break; case PUT: Put put = ProtobufUtil.toPut(mutation, cellScanner); + checkCellSizeLimit(region, put); quota.addMutation(put); if (request.hasCondition()) { Condition condition = request.getCondition(); @@ -2633,6 +2663,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, break; case DELETE: Delete delete = ProtobufUtil.toDelete(mutation, cellScanner); + checkCellSizeLimit(region, delete); quota.addMutation(delete); if (request.hasCondition()) { Condition condition = request.getCondition(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java index 7668aa251c..b3ac20ba24 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java @@ -6291,4 +6291,36 @@ public class TestFromClientSide { .getNumberOfCachedRegionLocations(htd.getTableName()); assertEquals(results.size(), number); } + + @Test + public void testCellSizeLimit() throws IOException { + final TableName tableName = TableName.valueOf("testCellSizeLimit"); + HTableDescriptor htd = new HTableDescriptor(tableName); + htd.setConfiguration(HConstants.HBASE_MAX_CELL_SIZE_KEY, Integer.toString(10 * 1024)); // 10K + HColumnDescriptor fam = new HColumnDescriptor(FAMILY); + htd.addFamily(fam); + Admin admin = TEST_UTIL.getAdmin(); + admin.createTable(htd); + // Will succeed + try (Table t = TEST_UTIL.getConnection().getTable(tableName)) { + t.put(new Put(ROW).addColumn(FAMILY, QUALIFIER, Bytes.toBytes(0L))); + t.increment(new Increment(ROW).addColumn(FAMILY, QUALIFIER, 1L)); + } + // Will fail + try (Table t = TEST_UTIL.getConnection().getTable(tableName)) { + try { + t.put(new Put(ROW).addColumn(FAMILY, QUALIFIER, new byte[10 * 1024])); + fail("Oversize cell failed to trigger exception"); + } catch (IOException e) { + // expected + } + try { + t.append(new Append(ROW).add(FAMILY, QUALIFIER, new byte[10 * 1024])); + fail("Oversize cell failed to trigger exception"); + } catch (IOException e) { + // expected + } + } + } + } -- 2.12.0