You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by la...@apache.org on 2012/08/25 06:32:22 UTC
svn commit: r1377205 - in /hbase/branches/0.94/src:
main/java/org/apache/hadoop/hbase/regionserver/
main/java/org/apache/hadoop/hbase/regionserver/metrics/
test/java/org/apache/hadoop/hbase/coprocessor/
test/java/org/apache/hadoop/hbase/regionserver/
Author: larsh
Date: Sat Aug 25 04:32:21 2012
New Revision: 1377205
URL: http://svn.apache.org/viewvc?rev=1377205&view=rev
Log:
HBASE-5292 getsize per-CF metric incorrectly counts compaction related reads as well
Modified:
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java
hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java
hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sat Aug 25 04:32:21 2012
@@ -44,7 +44,6 @@ import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.CompletionService;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
@@ -199,7 +198,7 @@ public class HRegion implements HeapSize
// Registered region protocol handlers
private ClassToInstanceMap<CoprocessorProtocol>
protocolHandlers = MutableClassToInstanceMap.create();
-
+
private Map<String, Class<? extends CoprocessorProtocol>>
protocolHandlerNames = Maps.newHashMap();
@@ -333,7 +332,6 @@ public class HRegion implements HeapSize
private RegionSplitPolicy splitPolicy;
private final OperationMetrics opMetrics;
-
/**
* Should only be used for testing purposes
*/
@@ -914,7 +912,7 @@ public class HRegion implements HeapSize
CompletionService<ImmutableList<StoreFile>> completionService =
new ExecutorCompletionService<ImmutableList<StoreFile>>(
storeCloserThreadPool);
-
+
// close each store in parallel
for (final Store store : stores.values()) {
completionService
@@ -2943,7 +2941,7 @@ public class HRegion implements HeapSize
return currentEditSeqId;
} finally {
status.cleanup();
- if (reader != null) {
+ if (reader != null) {
reader.close();
}
}
@@ -3389,6 +3387,12 @@ public class HRegion implements HeapSize
@Override
public synchronized boolean next(List<KeyValue> outResults, int limit)
throws IOException {
+ return next(outResults, limit, null);
+ }
+
+ @Override
+ public synchronized boolean next(List<KeyValue> outResults, int limit,
+ String metric) throws IOException {
if (this.filterClosed) {
throw new UnknownScannerException("Scanner was closed (timed out?) " +
"after we renewed it. Could be caused by a very slow scanner " +
@@ -3403,7 +3407,7 @@ public class HRegion implements HeapSize
results.clear();
- boolean returnResult = nextInternal(limit);
+ boolean returnResult = nextInternal(limit, metric);
outResults.addAll(results);
resetFilters();
@@ -3420,7 +3424,14 @@ public class HRegion implements HeapSize
public synchronized boolean next(List<KeyValue> outResults)
throws IOException {
// apply the batching limit by default
- return next(outResults, batch);
+ return next(outResults, batch, null);
+ }
+
+ @Override
+ public synchronized boolean next(List<KeyValue> outResults, String metric)
+ throws IOException {
+ // apply the batching limit by default
+ return next(outResults, batch, metric);
}
/*
@@ -3430,7 +3441,7 @@ public class HRegion implements HeapSize
return this.filter != null && this.filter.filterAllRemaining();
}
- private boolean nextInternal(int limit) throws IOException {
+ private boolean nextInternal(int limit, String metric) throws IOException {
RpcCallContext rpcCall = HBaseServer.getCurrentCall();
while (true) {
if (rpcCall != null) {
@@ -3457,7 +3468,7 @@ public class HRegion implements HeapSize
} else {
byte [] nextRow;
do {
- this.storeHeap.next(results, limit - results.size());
+ this.storeHeap.next(results, limit - results.size(), metric);
if (limit > 0 && results.size() == limit) {
if (this.filter != null && filter.hasFilterRow()) {
throw new IncompatibleFilterException(
@@ -4169,7 +4180,7 @@ public class HRegion implements HeapSize
RegionScanner scanner = null;
try {
scanner = getScanner(scan);
- scanner.next(results);
+ scanner.next(results, SchemaMetrics.METRIC_GETSIZE);
} finally {
if (scanner != null)
scanner.close();
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Sat Aug 25 04:32:21 2012
@@ -261,7 +261,6 @@ public class HRegionServer implements HR
private RegionServerMetrics metrics;
- @SuppressWarnings("unused")
private RegionServerDynamicMetrics dynamicMetrics;
// Compactions
@@ -308,7 +307,6 @@ public class HRegionServer implements HR
// Instance of the hbase executor service.
private ExecutorService service;
- @SuppressWarnings("unused")
// Replication services. If no replication, this handler will be null.
private ReplicationSourceService replicationSourceHandler;
@@ -2390,7 +2388,7 @@ public class HRegionServer implements HR
&& currentScanResultSize < maxScannerResultSize; i++) {
requestCount.incrementAndGet();
// Collect values to be returned here
- boolean moreRows = s.next(values);
+ boolean moreRows = s.next(values, SchemaMetrics.METRIC_NEXTSIZE);
if (!values.isEmpty()) {
for (KeyValue kv : values) {
currentScanResultSize += kv.heapSize();
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java Sat Aug 25 04:32:21 2012
@@ -47,6 +47,15 @@ public interface InternalScanner extends
* @throws IOException e
*/
public boolean next(List<KeyValue> results) throws IOException;
+
+ /**
+ * Grab the next row's worth of values.
+ * @param results return output array
+ * @param metric the metric name
+ * @return true if more rows exist after this one, false if scanner is done
+ * @throws IOException e
+ */
+ public boolean next(List<KeyValue> results, String metric) throws IOException;
/**
* Grab the next row's worth of values with a limit on the number of values
@@ -57,6 +66,17 @@ public interface InternalScanner extends
* @throws IOException e
*/
public boolean next(List<KeyValue> result, int limit) throws IOException;
+
+ /**
+ * Grab the next row's worth of values with a limit on the number of values
+ * to return.
+ * @param result return output array
+ * @param limit limit on row count to get
+ * @param metric the metric name
+ * @return true if more rows exist after this one, false if scanner is done
+ * @throws IOException e
+ */
+ public boolean next(List<KeyValue> result, int limit, String metric) throws IOException;
/**
* Closes the scanner and releases any resources it has allocated
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java Sat Aug 25 04:32:21 2012
@@ -120,11 +120,27 @@ public class KeyValueHeap extends NonLaz
* @return true if there are more keys, false if all scanners are done
*/
public boolean next(List<KeyValue> result, int limit) throws IOException {
+ return next(result, limit, null);
+ }
+
+ /**
+ * Gets the next row of keys from the top-most scanner.
+ * <p>
+ * This method takes care of updating the heap.
+ * <p>
+ * This can ONLY be called when you are using Scanners that implement
+ * InternalScanner as well as KeyValueScanner (a {@link StoreScanner}).
+ * @param result output result list
+ * @param limit limit on row count to get
+ * @param metric the metric name
+ * @return true if there are more keys, false if all scanners are done
+ */
+ public boolean next(List<KeyValue> result, int limit, String metric) throws IOException {
if (this.current == null) {
return false;
}
InternalScanner currentAsInternal = (InternalScanner)this.current;
- boolean mayContainMoreRows = currentAsInternal.next(result, limit);
+ boolean mayContainMoreRows = currentAsInternal.next(result, limit, metric);
KeyValue pee = this.current.peek();
/*
* By definition, any InternalScanner must return false only when it has no
@@ -156,6 +172,11 @@ public class KeyValueHeap extends NonLaz
return next(result, -1);
}
+ @Override
+ public boolean next(List<KeyValue> result, String metric) throws IOException {
+ return next(result, -1, metric);
+ }
+
private static class KVScannerComparator implements Comparator<KeyValueScanner> {
private KVComparator kvComparator;
/**
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java Sat Aug 25 04:32:21 2012
@@ -50,8 +50,8 @@ public class StoreScanner extends NonLaz
private KeyValueHeap heap;
private boolean cacheBlocks;
- private String metricNameGetSize;
+ private String metricNamePrefix;
// Used to indicate that the scanner has closed (see HBASE-1107)
// Doesnt need to be volatile because it's always accessed via synchronized methods
private boolean closing = false;
@@ -198,7 +198,7 @@ public class StoreScanner extends NonLaz
/**
* Method used internally to initialize metric names throughout the
* constructors.
- *
+ *
* To be called after the store variable has been initialized!
*/
private void initializeMetricNames() {
@@ -208,8 +208,8 @@ public class StoreScanner extends NonLaz
tableName = store.getTableName();
family = Bytes.toString(store.getFamily().getName());
}
- metricNameGetSize = SchemaMetrics.generateSchemaMetricsPrefix(
- tableName, family) + "getsize";
+ this.metricNamePrefix =
+ SchemaMetrics.generateSchemaMetricsPrefix(tableName, family);
}
/**
@@ -308,6 +308,18 @@ public class StoreScanner extends NonLaz
*/
@Override
public synchronized boolean next(List<KeyValue> outResult, int limit) throws IOException {
+ return next(outResult, limit, null);
+ }
+
+ /**
+ * Get the next row of values from this Store.
+ * @param outResult
+ * @param limit
+ * @return true if there are more rows, false if scanner is done
+ */
+ @Override
+ public synchronized boolean next(List<KeyValue> outResult, int limit,
+ String metric) throws IOException {
if (checkReseek()) {
return true;
@@ -420,7 +432,10 @@ public class StoreScanner extends NonLaz
}
}
} finally {
- RegionMetricsStorage.incrNumericMetric(metricNameGetSize, cumulativeMetric);
+ if (cumulativeMetric > 0 && metric != null) {
+ RegionMetricsStorage.incrNumericMetric(this.metricNamePrefix + metric,
+ cumulativeMetric);
+ }
}
if (!results.isEmpty()) {
@@ -436,7 +451,13 @@ public class StoreScanner extends NonLaz
@Override
public synchronized boolean next(List<KeyValue> outResult) throws IOException {
- return next(outResult, -1);
+ return next(outResult, -1, null);
+ }
+
+ @Override
+ public synchronized boolean next(List<KeyValue> outResult, String metric)
+ throws IOException {
+ return next(outResult, -1, metric);
}
// Implementation of ChangedReadersObserver
Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/SchemaMetrics.java Sat Aug 25 04:32:21 2012
@@ -176,6 +176,9 @@ public class SchemaMetrics {
/** Use for readability when obtaining non-compaction counters */
public static final boolean NO_COMPACTION = false;
+ public static final String METRIC_GETSIZE = "getsize";
+ public static final String METRIC_NEXTSIZE = "nextsize";
+
/**
* A special schema metric value that means "all tables aggregated" or
* "all column families aggregated" when used as a table name or a column
Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java Sat Aug 25 04:32:21 2012
@@ -66,11 +66,23 @@ public class TestCoprocessorInterface ex
public boolean next(List<KeyValue> results) throws IOException {
return delegate.next(results);
}
+
+ @Override
+ public boolean next(List<KeyValue> results, String metric)
+ throws IOException {
+ return delegate.next(results, metric);
+ }
@Override
public boolean next(List<KeyValue> result, int limit) throws IOException {
return delegate.next(result, limit);
}
+
+ @Override
+ public boolean next(List<KeyValue> result, int limit, String metric)
+ throws IOException {
+ return delegate.next(result, limit, metric);
+ }
@Override
public void close() throws IOException {
Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java Sat Aug 25 04:32:21 2012
@@ -304,13 +304,26 @@ public class TestRegionObserverInterface
public boolean next(List<KeyValue> results) throws IOException {
return next(results, -1);
}
+
+ @Override
+ public boolean next(List<KeyValue> results, String metric)
+ throws IOException {
+ return next(results, -1, metric);
+ }
+
+ @Override
+ public boolean next(List<KeyValue> results, int limit)
+ throws IOException{
+ return next(results, limit, null);
+ }
@Override
- public boolean next(List<KeyValue> results, int limit) throws IOException {
+ public boolean next(List<KeyValue> results, int limit, String metric)
+ throws IOException {
List<KeyValue> internalResults = new ArrayList<KeyValue>();
boolean hasMore;
do {
- hasMore = scanner.next(internalResults, limit);
+ hasMore = scanner.next(internalResults, limit, metric);
if (!internalResults.isEmpty()) {
long row = Bytes.toLong(internalResults.get(0).getRow());
if (row % 2 == 0) {
Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java?rev=1377205&r1=1377204&r2=1377205&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java Sat Aug 25 04:32:21 2012
@@ -29,6 +29,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.MediumTests;
import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.Append;
@@ -37,6 +38,7 @@ import org.apache.hadoop.hbase.client.Ge
import org.apache.hadoop.hbase.client.HTable;
import org.apache.hadoop.hbase.client.Increment;
import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.regionserver.metrics.RegionMetricsStorage;
import org.apache.hadoop.hbase.regionserver.metrics.SchemaMetrics;
import org.apache.hadoop.hbase.regionserver.metrics.SchemaMetrics.
@@ -69,7 +71,7 @@ public class TestRegionServerMetrics {
private static final SchemaMetrics ALL_METRICS =
SchemaMetrics.ALL_SCHEMA_METRICS;
- private static final HBaseTestingUtility TEST_UTIL =
+ private final HBaseTestingUtility TEST_UTIL =
new HBaseTestingUtility();
private Map<String, Long> startingMetrics;
@@ -270,5 +272,80 @@ public class TestRegionServerMetrics {
@org.junit.Rule
public org.apache.hadoop.hbase.ResourceCheckerJUnitRule cu =
new org.apache.hadoop.hbase.ResourceCheckerJUnitRule();
+
+ private void assertSizeMetric(String table, String[] cfs, int[] metrics) {
+ // we have getsize & nextsize for each column family
+ assertEquals(cfs.length * 2, metrics.length);
+
+ for (int i =0; i < cfs.length; ++i) {
+ String prefix = SchemaMetrics.generateSchemaMetricsPrefix(table, cfs[i]);
+ String getMetric = prefix + SchemaMetrics.METRIC_GETSIZE;
+ String nextMetric = prefix + SchemaMetrics.METRIC_NEXTSIZE;
+
+ // verify getsize and nextsize matches
+ int getSize = RegionMetricsStorage.getNumericMetrics().containsKey(getMetric) ?
+ RegionMetricsStorage.getNumericMetrics().get(getMetric).intValue() : 0;
+ int nextSize = RegionMetricsStorage.getNumericMetrics().containsKey(nextMetric) ?
+ RegionMetricsStorage.getNumericMetrics().get(nextMetric).intValue() : 0;
+
+ assertEquals(metrics[i], getSize);
+ assertEquals(metrics[cfs.length + i], nextSize);
+ }
+ }
+
+ @Test
+ public void testGetNextSize() throws IOException, InterruptedException {
+ String rowName = "row1";
+ byte[] ROW = Bytes.toBytes(rowName);
+ String tableName = "SizeMetricTest";
+ byte[] TABLE = Bytes.toBytes(tableName);
+ String cf1Name = "cf1";
+ String cf2Name = "cf2";
+ String[] cfs = new String[] {cf1Name, cf2Name};
+ byte[] CF1 = Bytes.toBytes(cf1Name);
+ byte[] CF2 = Bytes.toBytes(cf2Name);
+
+ long ts = 1234;
+ HTable hTable = TEST_UTIL.createTable(TABLE, new byte[][]{CF1, CF2});
+ HBaseAdmin admin = new HBaseAdmin(TEST_UTIL.getConfiguration());
+
+ Put p = new Put(ROW);
+ p.add(CF1, CF1, ts, CF1);
+ p.add(CF2, CF2, ts, CF2);
+ hTable.put(p);
+
+ KeyValue kv1 = new KeyValue(ROW, CF1, CF1, ts, CF1);
+ KeyValue kv2 = new KeyValue(ROW, CF2, CF2, ts, CF2);
+ int kvLength = kv1.getLength();
+ assertEquals(kvLength, kv2.getLength());
+
+ // only cf1.getsize is set on Get
+ hTable.get(new Get(ROW).addFamily(CF1));
+ assertSizeMetric(tableName, cfs, new int[] {kvLength, 0, 0, 0});
+
+ // only cf2.getsize is set on Get
+ hTable.get(new Get(ROW).addFamily(CF2));
+ assertSizeMetric(tableName, cfs, new int[] {kvLength, kvLength, 0, 0});
+
+ // only cf2.nextsize is set
+ for (Result res : hTable.getScanner(CF2)) {
+ }
+ assertSizeMetric(tableName, cfs,
+ new int[] {kvLength, kvLength, 0, kvLength});
+
+ // only cf2.nextsize is set
+ for (Result res : hTable.getScanner(CF1)) {
+ }
+ assertSizeMetric(tableName, cfs,
+ new int[] {kvLength, kvLength, kvLength, kvLength});
+
+ // getsize/nextsize should not be set on flush or compaction
+ for (HRegion hr : TEST_UTIL.getMiniHBaseCluster().getRegions(TABLE)) {
+ hr.flushcache();
+ hr.compactStores();
+ }
+ assertSizeMetric(tableName, cfs,
+ new int[] {kvLength, kvLength, kvLength, kvLength});
+ }
}