From a3842caf77cfadefee87b5ce3e5397534d15d9e6 Mon Sep 17 00:00:00 2001 From: anoopsjohn Date: Fri, 30 Oct 2015 18:41:45 +0530 Subject: [PATCH] HBASE-14721 Memstore add cells - Avoid many garbage. --- .../hbase/regionserver/DefaultMemStore.java | 18 ++++++----------- .../hadoop/hbase/regionserver/HRegion.java | 20 ++++++------------- .../hadoop/hbase/regionserver/HStore.java | 3 +-- .../hadoop/hbase/regionserver/MemStore.java | 6 ++---- .../hadoop/hbase/regionserver/Store.java | 5 ++--- .../hadoop/hbase/regionserver/TestStore.java | 17 ++++++---------- 6 files changed, 23 insertions(+), 46 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java index 13efee149cf..e542344c23a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java @@ -45,7 +45,6 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.ClassSize; import org.apache.hadoop.hbase.util.CollectionBackedScanner; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.ReflectionUtils; import org.apache.htrace.Trace; @@ -221,13 +220,12 @@ public class DefaultMemStore implements MemStore { /** * Write an update * @param cell - * @return approximate size of the passed KV & newly added KV which maybe different than the - * passed-in KV + * @return approximate size of the passed cell. */ @Override - public Pair add(Cell cell) { + public long add(Cell cell) { Cell toAdd = maybeCloneWithAllocator(cell); - return new Pair(internalAdd(toAdd), toAdd); + return internalAdd(toAdd); } @Override @@ -1082,21 +1080,17 @@ public class DefaultMemStore implements MemStore { byte [] empty = new byte[0]; for (int i = 0; i < count; i++) { // Give each its own ts - Pair ret = memstore1.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, empty)); - size += ret.getFirst(); + size += memstore1.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, empty)); } LOG.info("memstore1 estimated size=" + size); for (int i = 0; i < count; i++) { - Pair ret = memstore1.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, empty)); - size += ret.getFirst(); + size += memstore1.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, empty)); } LOG.info("memstore1 estimated size (2nd loading of same data)=" + size); // Make a variably sized memstore. DefaultMemStore memstore2 = new DefaultMemStore(); for (int i = 0; i < count; i++) { - Pair ret = memstore2.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, - new byte[i])); - size += ret.getFirst(); + size += memstore2.add(new KeyValue(Bytes.toBytes(i), fam, qf, i, new byte[i])); } LOG.info("memstore2 estimated size=" + size); final int seconds = 30; 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 f76bd5d14b0..682accac2da 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 @@ -3708,15 +3708,10 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi int listSize = cells.size(); for (int i=0; i < listSize; i++) { Cell cell = cells.get(i); - if (cell.getSequenceId() == 0) { + if (cell.getSequenceId() == 0 || isInReplay) { CellUtil.setSequenceId(cell, mvccNum); } - Pair ret = store.add(cell); - size += ret.getFirst(); - if(isInReplay) { - // set memstore newly added cells with replay mvcc number - CellUtil.setSequenceId(ret.getSecond(), mvccNum); - } + size += store.add(cell); } } @@ -4959,7 +4954,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi * @return True if we should flush. */ protected boolean restoreEdit(final Store s, final Cell cell) { - long kvSize = s.add(cell).getFirst(); + long kvSize = s.add(cell); if (this.rsAccounting != null) { rsAccounting.addAndGetRegionReplayEditsSize(getRegionInfo().getRegionName(), kvSize); } @@ -6913,8 +6908,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi checkFamily(CellUtil.cloneFamily(cell)); // unreachable } - Pair ret = store.add(cell); - addedSize += ret.getFirst(); + addedSize += store.add(cell); } } @@ -7262,8 +7256,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // otherwise keep older versions around for (Cell cell: entry.getValue()) { CellUtil.setSequenceId(cell, writeEntry.getWriteNumber()); - Pair ret = store.add(cell); - size += ret.getFirst(); + size += store.add(cell); doRollBackMemstore = true; } } @@ -7493,8 +7486,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // otherwise keep older versions around for (Cell cell : entry.getValue()) { CellUtil.setSequenceId(cell, writeEntry.getWriteNumber()); - Pair ret = store.add(cell); - size += ret.getFirst(); + size += store.add(cell); doRollBackMemstore = true; } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java index 595c76da8c4..8f061a54303 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java @@ -90,7 +90,6 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.ChecksumType; import org.apache.hadoop.hbase.util.ClassSize; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; -import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.ReflectionUtils; import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix; @@ -655,7 +654,7 @@ public class HStore implements Store { } @Override - public Pair add(final Cell cell) { + public long add(final Cell cell) { lock.readLock().lock(); try { return this.memstore.add(cell); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java index 364b9c9b639..658ba488eac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java @@ -22,7 +22,6 @@ import java.util.List; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.io.HeapSize; -import org.apache.hadoop.hbase.util.Pair; /** * The MemStore holds in-memory modifications to the Store. Modifications are {@link Cell}s. @@ -68,10 +67,9 @@ public interface MemStore extends HeapSize { /** * Write an update * @param cell - * @return approximate size of the passed KV and the newly added KV which maybe different from the - * passed in KV. + * @return approximate size of the passed cell. */ - Pair add(final Cell cell); + long add(final Cell cell); /** * @return Oldest timestamp of all the Cells in the MemStore diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java index 9fb25e7ffcd..9f1752626a8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java @@ -44,7 +44,6 @@ import org.apache.hadoop.hbase.regionserver.compactions.CompactionProgress; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequest; import org.apache.hadoop.hbase.regionserver.compactions.CompactionThroughputController; import org.apache.hadoop.hbase.security.User; -import org.apache.hadoop.hbase.util.Pair; /** * Interface for objects that hold a column family in a Region. Its a memstore and a set of zero or @@ -127,9 +126,9 @@ public interface Store extends HeapSize, StoreConfigInformation, PropagatingConf /** * Adds a value to the memstore * @param cell - * @return memstore size delta & newly added KV which maybe different than the passed in KV + * @return memstore size delta */ - Pair add(Cell cell); + long add(Cell cell); /** * When was the last edit done in the memstore diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java index 60a15ea3131..6c3f946c409 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java @@ -220,7 +220,7 @@ public class TestStore { long size = store.memstore.getFlushableSize(); Assert.assertEquals(0, size); LOG.info("Adding some data"); - long kvSize = store.add(new KeyValue(row, family, qf1, 1, (byte[])null)).getFirst(); + long kvSize = store.add(new KeyValue(row, family, qf1, 1, (byte[])null)); size = store.memstore.getFlushableSize(); Assert.assertEquals(kvSize, size); // Flush. Bug #1 from HBASE-10466. Make sure size calculation on failed flush is right. @@ -635,20 +635,15 @@ public class TestStore { size += this.store.add(new KeyValue(Bytes.toBytes("200909091000"), family, qf1, - System.currentTimeMillis(), - Bytes.toBytes(newValue))).getFirst(); + System.currentTimeMillis(), Bytes.toBytes(newValue))); size += this.store.add(new KeyValue(Bytes.toBytes("200909091200"), family, qf1, - System.currentTimeMillis(), - Bytes.toBytes(newValue))).getFirst(); + System.currentTimeMillis(), Bytes.toBytes(newValue))); size += this.store.add(new KeyValue(Bytes.toBytes("200909091300"), family, qf1, - System.currentTimeMillis(), - Bytes.toBytes(newValue))).getFirst(); + System.currentTimeMillis(), Bytes.toBytes(newValue))); size += this.store.add(new KeyValue(Bytes.toBytes("200909091400"), family, qf1, - System.currentTimeMillis(), - Bytes.toBytes(newValue))).getFirst(); + System.currentTimeMillis(), Bytes.toBytes(newValue))); size += this.store.add(new KeyValue(Bytes.toBytes("200909091500"), family, qf1, - System.currentTimeMillis(), - Bytes.toBytes(newValue))).getFirst(); + System.currentTimeMillis(), Bytes.toBytes(newValue))); for ( int i = 0 ; i < 10000 ; ++i) {