You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by mb...@apache.org on 2012/04/08 12:38:17 UTC
svn commit: r1310973 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java
main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java
test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java
Author: mbautin
Date: Sun Apr 8 10:38:17 2012
New Revision: 1310973
URL: http://svn.apache.org/viewvc?rev=1310973&view=rev
Log:
[jira] [HBASE-5744] [89-fb] Thrift server metrics should be long instead of int
Summary: As we measure our Thrift call latencies in nanoseconds, we need to make
latencies long instead of int everywhere. There is a bug where we truncate a
nanosecond latency to int, which is a problem with RPCs that take more than
2.147483647 seconds to process.
Test Plan:
TestThriftServer is updated to test for the failure case (an RPC is artificially
made to take 3 seconds). The new test case fails without the fix.
Re-run all unit tests.
Reviewers: schen, dhruba, kannan, liyintang
Reviewed By: schen
CC: stack
Differential Revision: https://reviews.facebook.net/D2679
Modified:
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java
hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java?rev=1310973&r1=1310972&r2=1310973&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/HbaseHandlerMetricsProxy.java Sun Apr 8 10:38:17 2012
@@ -63,7 +63,7 @@ public class HbaseHandlerMetricsProxy im
try {
long start = now();
result = m.invoke(handler, args);
- int processTime = (int)(now() - start);
+ long processTime = now() - start;
metrics.incMethodTime(m.getName(), processTime);
} catch (InvocationTargetException e) {
throw e.getTargetException();
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java?rev=1310973&r1=1310972&r2=1310973&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/thrift/ThriftMetrics.java Sun Apr 8 10:38:17 2012
@@ -101,16 +101,16 @@ public class ThriftMetrics implements Up
numBatchMutateRowKeys.inc(diff);
}
- public void incMethodTime(String name, int time) {
- MetricsTimeVaryingRate methodTimeMetrc = getMethodTimeMetrics(name);
- if (methodTimeMetrc == null) {
+ public void incMethodTime(String name, long time) {
+ MetricsTimeVaryingRate methodTimeMetric = getMethodTimeMetrics(name);
+ if (methodTimeMetric == null) {
LOG.warn(
"Got incMethodTime() request for method that doesnt exist: " + name);
return; // ignore methods that dont exist.
}
// inc method specific processTime
- methodTimeMetrc.inc(time);
+ methodTimeMetric.inc(time);
// inc general processTime
thriftCall.inc(time);
Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java?rev=1310973&r1=1310972&r2=1310973&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/thrift/TestThriftServer.java Sun Apr 8 10:38:17 2012
@@ -28,10 +28,12 @@ import org.apache.hadoop.hbase.HBaseClus
import org.apache.hadoop.hbase.thrift.generated.BatchMutation;
import org.apache.hadoop.hbase.thrift.generated.ColumnDescriptor;
import org.apache.hadoop.hbase.thrift.generated.Hbase;
+import org.apache.hadoop.hbase.thrift.generated.IOError;
import org.apache.hadoop.hbase.thrift.generated.Mutation;
import org.apache.hadoop.hbase.thrift.generated.TCell;
import org.apache.hadoop.hbase.thrift.generated.TRowResult;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.Threads;
import org.apache.hadoop.metrics.ContextFactory;
import org.apache.hadoop.metrics.MetricsContext;
import org.apache.hadoop.metrics.MetricsUtil;
@@ -105,12 +107,27 @@ public class TestThriftServer extends HB
handler.deleteTable(tableAname);
}
+ public static final class MySlowHBaseHandler extends ThriftServer.HBaseHandler
+ implements Hbase.Iface {
+
+ protected MySlowHBaseHandler(Configuration c, ThriftMetrics metrics)
+ throws IOException {
+ super(c, metrics);
+ }
+
+ @Override
+ public List<byte[]> getTableNames() throws IOError {
+ Threads.sleepWithoutInterrupt(3000);
+ return super.getTableNames();
+ }
+ }
+
/**
* Tests if the metrics for thrift handler work correctly
*/
public void doTestThriftMetrics() throws Exception {
ThriftMetrics metrics = getMetrics(conf);
- Hbase.Iface handler = getHandler(metrics, conf);
+ Hbase.Iface handler = getHandlerForMetricsTest(metrics, conf);
handler.createTable(tableAname, getColumnDescriptors());
handler.disableTable(tableAname);
handler.deleteTable(tableAname);
@@ -120,12 +137,16 @@ public class TestThriftServer extends HB
verifyMetrics(metrics, "createTable_num_ops", 2);
verifyMetrics(metrics, "deleteTable_num_ops", 2);
verifyMetrics(metrics, "disableTable_num_ops", 2);
+ handler.getTableNames(); // This will have an artificial delay.
+
+ // 3 to 6 seconds (to account for potential slowness), measured in nanoseconds.
+ verifyMetricRange(metrics, "getTableNames_avg_time", 3L * 1000 * 1000 * 1000,
+ 6L * 1000 * 1000 * 1000);
}
- private static Hbase.Iface getHandler(ThriftMetrics metrics, Configuration conf)
+ private static Hbase.Iface getHandlerForMetricsTest(ThriftMetrics metrics, Configuration conf)
throws Exception {
- Hbase.Iface handler =
- new ThriftServer.HBaseHandler(conf);
+ Hbase.Iface handler = new MySlowHBaseHandler(conf, metrics);
return HbaseHandlerMetricsProxy.newInstance(handler, metrics, conf);
}
@@ -143,14 +164,29 @@ public class TestThriftServer extends HB
.createRecord(ThriftMetrics.CONTEXT_NAME).remove();
}
- private static void verifyMetrics(ThriftMetrics metrics, String name, int expectValue)
+ private static void verifyMetrics(ThriftMetrics metrics, String name, long expectValue)
throws Exception {
+ long metricVal = getMetricValue(metrics, name);
+ assertEquals(expectValue, metricVal);
+ }
+
+ private static void verifyMetricRange(ThriftMetrics metrics, String name,
+ long minValue, long maxValue)
+ throws Exception {
+ long metricVal = getMetricValue(metrics, name);
+ if (metricVal < minValue || metricVal > maxValue) {
+ throw new AssertionError("Value of metric " + name + " is outside of the expected " +
+ "range [" + minValue + ", " + maxValue + "]: " + metricVal);
+ }
+ }
+
+ private static long getMetricValue(ThriftMetrics metrics, String name) {
MetricsContext context = MetricsUtil.getContext(
ThriftMetrics.CONTEXT_NAME);
metrics.doUpdates(context);
OutputRecord record = context.getAllRecords().get(
ThriftMetrics.CONTEXT_NAME).iterator().next();
- assertEquals(expectValue, record.getMetric(name).intValue());
+ return record.getMetric(name).longValue();
}
/**