From 18bff9047140f032dbcf17923dfab185e28a05a6 Mon Sep 17 00:00:00 2001 From: stack Date: Wed, 10 Dec 2014 23:17:30 -0800 Subject: [PATCH] commit 6ef97be15bb42d5f56f191e0263b172ff2debd5d Author: stack Date: Wed Dec 10 23:16:32 2014 -0800 HTRACE-13 Fix htrace-hbase circular dependencies + htrace-core/src/main/java/org/apache/htrace/SpanReceiverFactory.java Changed the 'build' method name to 'create'. When its 'build' i kept adding Builder pattern methods but this is a factory. Added extra 'create' that takes String of class to use creating an instance. + htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java + htrace-hbase/src/test/java/org/apache/htrace/impl/HBaseTestUtil.java Use new factory method creating spans. Formatting changes. --- .../org/apache/htrace/SpanReceiverFactory.java | 12 ++- .../org/apache/htrace/TestSpanReceiverFactory.java | 5 +- .../apache/htrace/HBaseHTraceConfiguration.java | 55 +++++++++++ .../org/apache/htrace/HBaseSpanReceiverHost.java | 106 +++++++++++++++++++++ .../org/apache/htrace/impl/HBaseSpanReceiver.java | 72 ++++++-------- .../java/org/apache/htrace/impl/HBaseTestUtil.java | 24 ++--- .../apache/htrace/impl/TestHBaseSpanReceiver.java | 20 ++-- 7 files changed, 217 insertions(+), 77 deletions(-) create mode 100644 htrace-hbase/src/main/java/org/apache/htrace/HBaseHTraceConfiguration.java create mode 100644 htrace-hbase/src/main/java/org/apache/htrace/HBaseSpanReceiverHost.java diff --git a/htrace-core/src/main/java/org/apache/htrace/SpanReceiverFactory.java b/htrace-core/src/main/java/org/apache/htrace/SpanReceiverFactory.java index 4d16ffb..7716ba3 100644 --- a/htrace-core/src/main/java/org/apache/htrace/SpanReceiverFactory.java +++ b/htrace-core/src/main/java/org/apache/htrace/SpanReceiverFactory.java @@ -55,11 +55,15 @@ public class SpanReceiverFactory { LOG.error(errorStr, e); } - public SpanReceiver build() { - String str = conf.get(SPAN_RECEIVER_CONF_KEY); - if ((str == null) || str.isEmpty()) { + public SpanReceiver create() { + return create(this.conf.get(SPAN_RECEIVER_CONF_KEY)); + } + + public SpanReceiver create(final String classToUseCreatingAnInstance) { + if ((classToUseCreatingAnInstance == null) || classToUseCreatingAnInstance.isEmpty()) { return null; } + String str = classToUseCreatingAnInstance; if (!str.contains(".")) { str = "org.apache.htrace.impl." + str; } @@ -92,4 +96,4 @@ public class SpanReceiverFactory { return null; } } -} +} \ No newline at end of file diff --git a/htrace-core/src/test/java/org/apache/htrace/TestSpanReceiverFactory.java b/htrace-core/src/test/java/org/apache/htrace/TestSpanReceiverFactory.java index 87b5085..18de6e1 100644 --- a/htrace-core/src/test/java/org/apache/htrace/TestSpanReceiverFactory.java +++ b/htrace-core/src/test/java/org/apache/htrace/TestSpanReceiverFactory.java @@ -23,7 +23,6 @@ import org.junit.Test; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.logging.Level; public class TestSpanReceiverFactory { /** @@ -33,7 +32,7 @@ public class TestSpanReceiverFactory { public void testGetNullSpanReceiver() { SpanReceiverFactory factory = new SpanReceiverFactory(HTraceConfiguration.EMPTY).logErrors(false); - SpanReceiver rcvr = factory.build(); + SpanReceiver rcvr = factory.create(); Assert.assertEquals(null, rcvr); } @@ -42,7 +41,7 @@ public class TestSpanReceiverFactory { SpanReceiverFactory factory = new SpanReceiverFactory(hconf). logErrors(false); - return factory.build(); + return factory.create(); } /** diff --git a/htrace-hbase/src/main/java/org/apache/htrace/HBaseHTraceConfiguration.java b/htrace-hbase/src/main/java/org/apache/htrace/HBaseHTraceConfiguration.java new file mode 100644 index 0000000..d6311d9 --- /dev/null +++ b/htrace-hbase/src/main/java/org/apache/htrace/HBaseHTraceConfiguration.java @@ -0,0 +1,55 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.htrace; + +import org.apache.hadoop.conf.Configuration; +import org.apache.htrace.HTraceConfiguration; + +/** + * Meshes {@link HTraceConfiguration} to {@link Configuration} + * @author stack + * + */ +public class HBaseHTraceConfiguration extends HTraceConfiguration { + public static final String KEY_PREFIX = "hbase."; + private final Configuration conf; + + public HBaseHTraceConfiguration(Configuration conf) { + this.conf = conf; + // Set default span receiver + this.conf.set(SpanReceiverFactory.SPAN_RECEIVER_CONF_KEY, + "org.apache.htrace.HBaseSpanReceiver"); + } + + @Override + public String get(String key) { + return conf.get(KEY_PREFIX +key); + } + + @Override + public String get(String key, String defaultValue) { + return conf.get(KEY_PREFIX + key,defaultValue); + + } + + @Override + public boolean getBoolean(String key, boolean defaultValue) { + return conf.getBoolean(KEY_PREFIX + key, defaultValue); + } +} \ No newline at end of file diff --git a/htrace-hbase/src/main/java/org/apache/htrace/HBaseSpanReceiverHost.java b/htrace-hbase/src/main/java/org/apache/htrace/HBaseSpanReceiverHost.java new file mode 100644 index 0000000..975cdba --- /dev/null +++ b/htrace-hbase/src/main/java/org/apache/htrace/HBaseSpanReceiverHost.java @@ -0,0 +1,106 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.htrace; + +import java.io.IOException; +import java.util.Collection; +import java.util.HashSet; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.htrace.SpanReceiver; +import org.apache.htrace.Trace; + +/** + * This class provides functions for reading the names of SpanReceivers from + * hbase-site.xml, adding those SpanReceivers to the Tracer, and closing those + * SpanReceivers when appropriate. + */ +public class HBaseSpanReceiverHost { + public static final String SPAN_RECEIVERS_CONF_KEY = "hbase.trace.spanreceiver.classes"; + private static final Log LOG = LogFactory.getLog(HBaseSpanReceiverHost.class); + private Collection receivers; + private Configuration conf; + private boolean closed = false; + + private static enum SingletonHolder { + INSTANCE; + Object lock = new Object(); + HBaseSpanReceiverHost host = null; + } + + public static HBaseSpanReceiverHost getInstance(Configuration conf) { + synchronized (SingletonHolder.INSTANCE.lock) { + if (SingletonHolder.INSTANCE.host != null) { + return SingletonHolder.INSTANCE.host; + } + + HBaseSpanReceiverHost host = new HBaseSpanReceiverHost(conf); + host.loadSpanReceivers(); + SingletonHolder.INSTANCE.host = host; + return SingletonHolder.INSTANCE.host; + } + + } + + HBaseSpanReceiverHost(Configuration conf) { + receivers = new HashSet(); + this.conf = conf; + } + + /** + * Reads the names of classes specified in the + * "hbase.trace.spanreceiver.classes" property and instantiates and registers + * them with the Tracer as SpanReceiver's. + * + */ + public void loadSpanReceivers() { + String[] receiverNames = conf.getStrings(SPAN_RECEIVERS_CONF_KEY); + if (receiverNames == null || receiverNames.length == 0) { + return; + } + SpanReceiverFactory factory = new SpanReceiverFactory(new HBaseHTraceConfiguration(this.conf)); + for (String className : receiverNames) { + className = className.trim(); + SpanReceiver receiver = factory.create(className); + if (receiver != null) { + receivers.add(receiver); + LOG.info("SpanReceiver " + className + " was loaded successfully."); + } + } + for (SpanReceiver rcvr : receivers) { + Trace.addReceiver(rcvr); + } + } + + /** + * Calls close() on all SpanReceivers created by this HBaseSpanReceiverHost. + */ + public synchronized void closeReceivers() { + if (closed) return; + closed = true; + for (SpanReceiver rcvr : receivers) { + try { + rcvr.close(); + } catch (IOException e) { + LOG.warn("Unable to close SpanReceiver correctly: " + e.getMessage(), e); + } + } + } +} \ No newline at end of file diff --git a/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java b/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java index 4a499ec..8f6a587 100644 --- a/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java +++ b/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java @@ -17,43 +17,40 @@ package org.apache.htrace.impl; -import com.google.common.util.concurrent.ThreadFactoryBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.commons.codec.binary.Base64; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HBaseConfiguration; -import org.apache.hadoop.hbase.client.HConnection; -import org.apache.hadoop.hbase.client.HConnectionManager; -import org.apache.hadoop.hbase.client.HTableInterface; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Put; -import org.apache.hadoop.hbase.trace.HBaseHTraceConfiguration; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.htrace.HBaseHTraceConfiguration; import org.apache.htrace.HTraceConfiguration; import org.apache.htrace.Sampler; import org.apache.htrace.Span; import org.apache.htrace.SpanReceiver; +import org.apache.htrace.SpanReceiverFactory; import org.apache.htrace.TimelineAnnotation; import org.apache.htrace.Trace; import org.apache.htrace.TraceScope; import org.apache.htrace.protobuf.generated.SpanProtos; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; +import com.google.common.util.concurrent.ThreadFactoryBuilder; /** * HBase is an open source distributed datastore. @@ -155,8 +152,8 @@ public class HBaseSpanReceiver implements SpanReceiver { } private class WriteSpanRunnable implements Runnable { - private HConnection hconnection; - private HTableInterface htable; + private Connection hconnection; + private Table htable; public WriteSpanRunnable() { } @@ -285,8 +282,8 @@ public class HBaseSpanReceiver implements SpanReceiver { private void startClient() { if (this.htable == null) { try { - hconnection = HConnectionManager.createConnection(hconf); - htable = hconnection.getTable(table); + hconnection = ConnectionFactory.createConnection(hconf); + htable = hconnection.getTable(TableName.valueOf(table)); } catch (IOException e) { LOG.warn("Failed to create HBase connection. " + e.getMessage()); } @@ -333,26 +330,20 @@ public class HBaseSpanReceiver implements SpanReceiver { /** * Run basic test. * @throws IOException - * - * - * TODO: !!!!! FIX !!!! Circular dependency back to HBase!!! - * + */ public static void main(String[] args) throws Exception { - HBaseSpanReceiver receiver = new HBaseSpanReceiver(); - receiver.configure(new HBaseHTraceConfiguration(HBaseConfiguration.create())); + SpanReceiverFactory factory = + new SpanReceiverFactory(new HBaseHTraceConfiguration(HBaseConfiguration.create())); + SpanReceiver receiver = factory.create(); Trace.addReceiver(receiver); - TraceScope parent = - Trace.startSpan("HBaseSpanReceiver.main.parent", Sampler.ALWAYS); + TraceScope parent = Trace.startSpan("HBaseSpanReceiver.main.parent", Sampler.ALWAYS); Thread.sleep(10); long traceid = parent.getSpan().getTraceId(); - TraceScope child1 = - Trace.startSpan("HBaseSpanReceiver.main.child.1"); + TraceScope child1 = Trace.startSpan("HBaseSpanReceiver.main.child.1"); Thread.sleep(10); - TraceScope child2 = - Trace.startSpan("HBaseSpanReceiver.main.child.2", parent.getSpan()); + TraceScope child2 = Trace.startSpan("HBaseSpanReceiver.main.child.2", parent.getSpan()); Thread.sleep(10); - TraceScope gchild = - Trace.startSpan("HBaseSpanReceiver.main.grandchild"); + TraceScope gchild = Trace.startSpan("HBaseSpanReceiver.main.grandchild"); Trace.addTimelineAnnotation("annotation 1."); Thread.sleep(10); Trace.addTimelineAnnotation("annotation 2."); @@ -365,5 +356,4 @@ public class HBaseSpanReceiver implements SpanReceiver { receiver.close(); System.out.println("trace id: " + traceid); } - */ -} +} \ No newline at end of file diff --git a/htrace-hbase/src/test/java/org/apache/htrace/impl/HBaseTestUtil.java b/htrace-hbase/src/test/java/org/apache/htrace/impl/HBaseTestUtil.java index bcc5d27..a34d563 100644 --- a/htrace-hbase/src/test/java/org/apache/htrace/impl/HBaseTestUtil.java +++ b/htrace-hbase/src/test/java/org/apache/htrace/impl/HBaseTestUtil.java @@ -19,24 +19,19 @@ package org.apache.htrace.impl; import java.io.IOException; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.client.HTableInterface; -import org.apache.hadoop.hbase.trace.HBaseHTraceConfiguration; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.htrace.HTraceConfiguration; +import org.apache.htrace.HBaseHTraceConfiguration; import org.apache.htrace.SpanReceiver; -import org.apache.htrace.impl.HBaseSpanReceiver; +import org.apache.htrace.SpanReceiverFactory; import org.junit.Assert; public class HBaseTestUtil { - private static final Log LOG = LogFactory.getLog(HBaseTestUtil.class); - public static Configuration configure(Configuration conf) { Configuration hconf = HBaseConfiguration.create(conf); hconf.set(HBaseHTraceConfiguration.KEY_PREFIX + @@ -51,8 +46,8 @@ public class HBaseTestUtil { return hconf; } - public static HTableInterface createTable(HBaseTestingUtility util) { - HTableInterface htable = null; + public static Table createTable(HBaseTestingUtility util) { + Table htable = null; try { htable = util.createTable(Bytes.toBytes(HBaseSpanReceiver.DEFAULT_TABLE), new byte[][]{Bytes.toBytes(HBaseSpanReceiver.DEFAULT_COLUMNFAMILY), @@ -64,12 +59,7 @@ public class HBaseTestUtil { } public static SpanReceiver startReceiver(Configuration conf) { - /* TODO: FIX!!!!! CIRCULAR DEPENDENCY BACK TO HBASE - SpanReceiver receiver = new HBaseSpanReceiver(); - receiver.configure(new HBaseHTraceConfiguration(conf)); - return receiver; - */ - return null; + return new SpanReceiverFactory(new HBaseHTraceConfiguration(conf)).create(); } public static SpanReceiver startReceiver(HBaseTestingUtility util) { @@ -86,4 +76,4 @@ public class HBaseTestUtil { } } } -} +} \ No newline at end of file diff --git a/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java b/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java index 4d6d15c..9fd3ca8 100644 --- a/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java +++ b/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java @@ -17,11 +17,9 @@ package org.apache.htrace.impl; -import com.google.common.collect.Multimap; - -import java.io.InputStream; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -32,9 +30,8 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.client.HTableInterface; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; @@ -42,15 +39,16 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.htrace.Span; import org.apache.htrace.SpanReceiver; import org.apache.htrace.TimelineAnnotation; +import org.apache.htrace.TraceCreator; import org.apache.htrace.TraceTree; -import org.apache.htrace.impl.HBaseSpanReceiver; import org.apache.htrace.protobuf.generated.SpanProtos; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.BeforeClass; -import org.junit.Test; import org.junit.Ignore; -import org.junit.Assert; -import org.apache.htrace.TraceCreator; +import org.junit.Test; + +import com.google.common.collect.Multimap; public class TestHBaseSpanReceiver { @@ -70,7 +68,7 @@ public class TestHBaseSpanReceiver { // Reenable after fix circular dependency @Ignore @Test public void testHBaseSpanReceiver() { - HTableInterface htable = HBaseTestUtil.createTable(UTIL); + Table htable = HBaseTestUtil.createTable(UTIL); SpanReceiver receiver = HBaseTestUtil.startReceiver(UTIL); TraceCreator tc = new TraceCreator(receiver); tc.createThreadedTrace(); @@ -108,7 +106,6 @@ public class TestHBaseSpanReceiver { Assert.assertTrue(descs.keySet().contains(TraceCreator.SIMPLE_TRACE_ROOT)); Assert.assertTrue(descs.keySet().contains(TraceCreator.THREADED_TRACE_ROOT)); - /** TODO: FIX!!!!!! Multimap spansByParentId = traceTree.getSpansByParentIdMap(); Span rpcRoot = descs.get(TraceCreator.RPC_TRACE_ROOT); Assert.assertEquals(1, spansByParentId.get(rpcRoot.getSpanId()).size()); @@ -137,7 +134,6 @@ public class TestHBaseSpanReceiver { } catch (IOException e) { Assert.fail("failed to get spans from index family. " + e.getMessage()); } - */ } private class TestSpan implements Span { -- 1.8.5.2 (Apple Git-48)