From 927729362f7a6cac33ad990b06314b9252dfb278 Mon Sep 17 00:00:00 2001 From: chenheng Date: Wed, 13 Jan 2016 11:13:35 +0800 Subject: [PATCH] HBASE-15095 isReturnResult=false on fast path in branch-1.1 and branch-1.0 is not respected --- .../org/apache/hadoop/hbase/client/Append.java | 1 - .../org/apache/hadoop/hbase/client/Increment.java | 12 +++++ .../org/apache/hadoop/hbase/client/Mutation.java | 2 + .../apache/hadoop/hbase/regionserver/HRegion.java | 4 +- .../hbase/client/TestIncrementsFromClientSide.java | 62 ++++++++++++---------- 5 files changed, 51 insertions(+), 30 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Append.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Append.java index 58c204b..1bdb121 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Append.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Append.java @@ -47,7 +47,6 @@ import org.apache.hadoop.hbase.util.Bytes; @InterfaceAudience.Public @InterfaceStability.Stable public class Append extends Mutation { - private static final String RETURN_RESULTS = "_rr_"; /** * @param returnResults * True (default) if the append operation should return the results. diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java index af0ea56..f090cf0 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java @@ -90,6 +90,18 @@ public class Increment extends Mutation implements Comparable { } /** + * @return current setting for returnResults + */ + public boolean isReturnResults() { + byte[] v = getAttribute(RETURN_RESULTS); + return v == null ? true : Bytes.toBoolean(v); + } + + public Increment setReturnResults(boolean returnResults) { + setAttribute(RETURN_RESULTS, Bytes.toBytes(returnResults)); + return this; + } + /** * Add the specified KeyValue to this operation. * @param cell individual Cell * @return this diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java index 2b88ffc..2444cf4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java @@ -83,6 +83,8 @@ public abstract class Mutation extends OperationWithAttributes implements Row, C */ private static final String OP_ATTRIBUTE_TTL = "_ttl"; + protected static final String RETURN_RESULTS = "_rr_"; + protected byte [] row = null; protected long ts = HConstants.LATEST_TIMESTAMP; protected Durability durability = Durability.USE_DEFAULT; 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 e655f4e..8df37ff 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 @@ -5893,7 +5893,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver { // try { if (this.coprocessorHost != null) { Result r = this.coprocessorHost.preIncrementAfterRowLock(increment); - if (r != null) return r; + if (r != null) return increment.isReturnResults() ? r : null; } // Process increments a Store/family at a time. long now = EnvironmentEdgeManager.currentTime(); @@ -5967,7 +5967,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver { // } // Request a cache flush. Do it outside update lock. if (isFlushSize(this.addAndGetGlobalMemstoreSize(accumulatedResultSize))) requestFlush(); - return Result.create(allKVs); + return increment.isReturnResults() ? Result.create(allKVs) : null; } private Result slowButConsistentIncrement(Increment increment, long nonceGroup, long nonce) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIncrementsFromClientSide.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIncrementsFromClientSide.java index f9461bc..77cebbd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIncrementsFromClientSide.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIncrementsFromClientSide.java @@ -18,9 +18,6 @@ */ package org.apache.hadoop.hbase.client; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; - import java.io.IOException; import java.util.Arrays; import java.util.Collection; @@ -35,12 +32,9 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.coprocessor.MultiRowMutationEndpoint; import org.apache.hadoop.hbase.regionserver.HRegion; -import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.Threads; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -51,6 +45,10 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.junit.Assert.assertTrue; + /** * Run Increment tests that use the HBase clients; {@link HTable}. * @@ -89,30 +87,18 @@ public class TestIncrementsFromClientSide { conf.setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, MultiRowMutationEndpoint.class.getName()); conf.setBoolean("hbase.table.sanity.checks", true); // enable for below tests - // We need more than one region server in this test - TEST_UTIL.startMiniCluster(SLAVES); } @Before public void before() throws Exception { Configuration conf = TEST_UTIL.getConfiguration(); if (this.fast) { - // If fast is set, set our configuration and then do a rolling restart of the one - // regionserver so it picks up the new config. Doing this should be faster than starting - // and stopping a cluster for each test. this.oldINCREMENT_FAST_BUT_NARROW_CONSISTENCY_KEY = conf.get(HRegion.INCREMENT_FAST_BUT_NARROW_CONSISTENCY_KEY); conf.setBoolean(HRegion.INCREMENT_FAST_BUT_NARROW_CONSISTENCY_KEY, this.fast); - HRegionServer rs = - TEST_UTIL.getHBaseCluster().getLiveRegionServerThreads().get(0).getRegionServer(); - TEST_UTIL.getHBaseCluster().startRegionServer(); - rs.stop("Restart"); - while(!rs.isStopped()) { - Threads.sleep(100); - LOG.info("Restarting " + rs); - } - TEST_UTIL.waitUntilNoRegionsInTransition(10000); } + // We need more than one region server in this test + TEST_UTIL.startMiniCluster(SLAVES); } @After @@ -124,13 +110,6 @@ public class TestIncrementsFromClientSide { this.oldINCREMENT_FAST_BUT_NARROW_CONSISTENCY_KEY); } } - } - - /** - * @throws java.lang.Exception - */ - @AfterClass - public static void afterClass() throws Exception { TEST_UTIL.shutdownMiniCluster(); } @@ -185,6 +164,35 @@ public class TestIncrementsFromClientSide { } @Test + public void testIncrementReturnValue() throws Exception { + LOG.info("Starting " + this.name.getMethodName()); + final TableName TABLENAME = + TableName.valueOf(filterStringSoTableNameSafe(this.name.getMethodName())); + Table ht = TEST_UTIL.createTable(TABLENAME, FAMILY); + final byte[] COLUMN = Bytes.toBytes("column"); + Put p = new Put(ROW); + p.add(FAMILY, COLUMN, Bytes.toBytes(5L)); + ht.put(p); + + Increment inc = new Increment(ROW); + inc.addColumn(FAMILY, COLUMN, 5L); + + Result r = ht.increment(inc); + long result = Bytes.toLong(r.getValue(FAMILY, COLUMN)); + assertEquals(10, result); + + if (this.fast) { + inc = new Increment(ROW); + inc.addColumn(FAMILY, COLUMN, 5L); + inc.setReturnResults(false); + r = ht.increment(inc); + assertTrue(r.getExists() == null); + } + + } + + + @Test public void testIncrementInvalidArguments() throws Exception { LOG.info("Starting " + this.name.getMethodName()); final TableName TABLENAME = -- 1.9.3 (Apple Git-50)