You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by li...@apache.org on 2014/03/01 20:18:25 UTC
svn commit: r1573210 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java
Author: liyin
Date: Sat Mar 1 19:18:25 2014
New Revision: 1573210
URL: http://svn.apache.org/r1573210
Log:
[HBASE-10647] Fix the bug of not decrease from size in updateColumnValue when removing the old columns.
Author: daviddeng
Summary:
In `MemStore.updateColumnValue`, when removing old values, `size` is not decreased, but local variable `addedSize` is decreased, so size is greater than real value. This leads to the problem of negative value for `HRegion.memstoreSize`.
Add a new case in `TestMemStore` to test it.
Test Plan: Run `TestMemStore`
Reviewers: manukranthk, aaiyer, rshroff, liyintang
Reviewed By: aaiyer
CC: hbase-eng@, arice
Differential Revision: https://phabricator.fb.com/D1197954
Task ID: 3831205
Modified:
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1573210&r1=1573209&r2=1573210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sat Mar 1 19:18:25 2014
@@ -629,6 +629,7 @@ public class HRegion implements HeapSize
for (final HColumnDescriptor family : families) {
status.setStatus("Instantiating store for column family " + family);
completionService.submit(new Callable<Store>() {
+ @Override
public Store call() throws IOException {
return instantiateHStore(tableDir, family);
}
@@ -767,13 +768,19 @@ public class HRegion implements HeapSize
/**
* Increase the size of mem store in this region and the sum of global mem
* stores' size
- * @param memStoreSize
+ * @param delta
* @return the size of memstore in this region
*/
- public long incMemoryUsage(long memStoreSize) {
+ public long incMemoryUsage(long delta) {
if (this.regionServer != null)
- this.regionServer.getGlobalMemstoreSize().addAndGet(memStoreSize);
- return this.memstoreSize.addAndGet(memStoreSize);
+ this.regionServer.getGlobalMemstoreSize().addAndGet(delta);
+ long vl = this.memstoreSize.addAndGet(delta);
+ if (vl < 0) {
+ LOG.error(String.format("memstoreSize of region %s becomes negative %d "
+ + "increased by %d called by %s", this.toString(), vl, delta, Thread
+ .currentThread().getStackTrace()[2]));
+ }
+ return vl;
}
/** @return a HRegionInfo object for this region */
@@ -922,6 +929,7 @@ public class HRegion implements HeapSize
assert store.getFlushableMemstoreSize() == 0;
completionService
.submit(new Callable<ImmutableList<StoreFile>>() {
+ @Override
public ImmutableList<StoreFile> call() throws IOException {
ImmutableList<StoreFile> result = store.close();
HRegionServer.configurationManager.
@@ -1501,6 +1509,11 @@ public class HRegion implements HeapSize
final long startTime = EnvironmentEdgeManager.currentTimeMillis();
// If nothing to flush, return and avoid logging start/stop flush.
if (this.memstoreSize.get() <= 0) {
+ if (this.memstoreSize.get() < 0) {
+ LOG.error(String.format(
+ "HRegion.memstoreSize is a negative number %ld for region %s",
+ memstoreSize.get(), this.toString()));
+ }
// Since there is nothing to flush, we will reset the flush times for all
// the stores.
for (Store store : stores.values()) {
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java?rev=1573210&r1=1573209&r2=1573210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java Sat Mar 1 19:18:25 2014
@@ -549,7 +549,9 @@ public class MemStore implements HeapSiz
if (kv.getType() == KeyValue.Type.Put.getCode()) {
// false means there was a change, so give us the size.
- addedSize -= heapSizeChange(kv, true);
+ long sz = heapSizeChange(kv, true);
+ addedSize -= sz;
+ this.size.addAndGet(-sz);
it.remove();
}
@@ -713,6 +715,7 @@ public class MemStore implements HeapSiz
return ret;
}
+ @Override
public synchronized boolean seek(KeyValue key) {
if (key == null) {
close();
@@ -777,11 +780,13 @@ public class MemStore implements HeapSiz
return (kvsetNextRow != null || snapshotNextRow != null);
}
+ @Override
public synchronized KeyValue peek() {
return getLowest();
}
+ @Override
public synchronized KeyValue next() {
KeyValue theNext = getLowest();
@@ -820,6 +825,7 @@ public class MemStore implements HeapSiz
return (first != null ? first : second);
}
+ @Override
public synchronized void close() {
this.kvsetNextRow = null;
this.snapshotNextRow = null;
Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java?rev=1573210&r1=1573209&r2=1573210&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestMemStore.java Sat Mar 1 19:18:25 2014
@@ -26,8 +26,6 @@ import java.rmi.UnexpectedException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.NavigableSet;
-import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicReference;
import junit.framework.TestCase;
@@ -38,17 +36,18 @@ import org.apache.hadoop.conf.Configurat
import org.apache.hadoop.hbase.HBaseConfiguration;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.KeyValue.Type;
import org.apache.hadoop.hbase.KeyValueTestUtil;
-import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.regionserver.kvaggregator.DefaultKeyValueAggregator;
import org.apache.hadoop.hbase.regionserver.metrics.SchemaMetrics;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.HasThread;
+import org.junit.Test;
import com.google.common.base.Joiner;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
-import org.apache.hadoop.hbase.util.HasThread;
/** memstore test case */
public class TestMemStore extends TestCase {
@@ -83,6 +82,19 @@ public class TestMemStore extends TestCa
found.getValue()));
}
+ @Test
+ public void testUpdateColumnValueSizeChange() {
+ byte[] bytes = Bytes.toBytes(getName());
+ KeyValue kv = new KeyValue(bytes, bytes, bytes, 0, Type.Put,
+ Bytes.toBytes((long) 0));
+ long size = this.memstore.size.get();
+ size += this.memstore.add(kv);
+ assertEquals("memstore.size()", size, memstore.size.get());
+
+ size += this.memstore.updateColumnValue(bytes, bytes, bytes, 1, 1, 1);
+ assertEquals("memstore.size()", size, memstore.size.get());
+ }
+
/**
* Test memstore snapshot happening while scanning.
* @throws IOException
@@ -406,6 +418,7 @@ public class TestMemStore extends TestCa
row = Bytes.toBytes(id);
}
+ @Override
public void run() {
try {
internalRun();
@@ -576,6 +589,7 @@ public class TestMemStore extends TestCa
// Clear out set. Otherwise row results accumulate.
results.clear();
}
+ scanner.close();
}
}
@@ -909,15 +923,6 @@ public class TestMemStore extends TestCa
}
}
- private KeyValue getDeleteKV(byte [] row) {
- return new KeyValue(row, Bytes.toBytes("test_col"), null,
- HConstants.LATEST_TIMESTAMP, KeyValue.Type.Delete, null);
- }
-
- private KeyValue getKV(byte [] row, byte [] value) {
- return new KeyValue(row, Bytes.toBytes("test_col"), null,
- HConstants.LATEST_TIMESTAMP, value);
- }
private static void addRows(int count, final MemStore mem) {
long nanos = System.nanoTime();