From 6ab8bd7a3b7b3dc39d8dcc79b9ef9a1d2ed026b1 Mon Sep 17 00:00:00 2001 From: Mikhail Bautin Date: Tue, 11 Sep 2012 12:50:30 +0000 Subject: [PATCH 2/2] [jira] [HBASE-6735] [89-fb] Remove reflection from HBaseClient Author: michalgr Summary: Every time we read response in HBaseClient we create instance of valueClass using reflection and Hadoop APIs. valueClass is set in a constructor of HBaseClient to one of parameters. The parameter is always HbaseObjectWritable. We can remove reflection by hardcoding HbaseObjectWritable. Test Plan: Unit tests. 3 fails: * TestDistributedLogSplitAtStartup * TestLogSplitOnMasterFailover * TestDistributedLogSplitting All 3 fails when run against trunk. Reviewers: kannan Reviewed By: kannan CC: Karthik, mbautin, Liyin, aaiyer Differential Revision: https://reviews.facebook.net/D5271 git-svn-id: https://svn.apache.org/repos/asf/hbase/branches/0.89-fb@1383389 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/hadoop/hbase/ipc/HBaseClient.java | 36 +++++++++++--------- .../java/org/apache/hadoop/hbase/ipc/HBaseRPC.java | 7 ++-- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java b/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java index ba48c9c..90ea2b4 100644 --- a/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java +++ b/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java @@ -54,6 +54,7 @@ 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.io.HbaseObjectWritable; import org.apache.hadoop.hbase.io.hfile.Compression; import org.apache.hadoop.io.DataOutputBuffer; import org.apache.hadoop.io.IOUtils; @@ -85,7 +86,6 @@ public class HBaseClient { protected final ConcurrentMap connections = new ConcurrentHashMap(); - protected final Class valueClass; // class of call values protected int counter; // counter for call ids protected final AtomicBoolean running = new AtomicBoolean(true); // if client runs final protected Configuration conf; @@ -137,7 +137,7 @@ public class HBaseClient { private class Call { final int id; // call id final Writable param; // parameter - Writable value; // value, null if error + HbaseObjectWritable value; // value, null if error IOException error; // exception, null if value boolean done; // true when call is done protected int version = HBaseServer.CURRENT_VERSION; @@ -173,7 +173,7 @@ public class HBaseClient { * * @param value return value of the call. */ - public synchronized void setValue(Writable value) { + public synchronized void setValue(HbaseObjectWritable value) { this.value = value; callComplete(); } @@ -616,7 +616,7 @@ public class HBaseClient { WritableUtils.readString(localIn))); calls.remove(id); } else { - Writable value = ReflectionUtils.newInstance(valueClass, conf); + HbaseObjectWritable value = createNewHbaseWritable(); value.readFields(localIn); // read value if (call.getVersion() >= HBaseServer.VERSION_RPCOPTIONS) { boolean hasProfiling = localIn.readBoolean(); @@ -651,6 +651,12 @@ public class HBaseClient { } } } + + private HbaseObjectWritable createNewHbaseWritable() { + HbaseObjectWritable ret = new HbaseObjectWritable(); + ret.setConf(conf); + return ret; + } private synchronized void markClosed(IOException e) { if (shouldCloseConnection.compareAndSet(false, true)) { @@ -729,12 +735,12 @@ public class HBaseClient { /** Result collector for parallel calls. */ private static class ParallelResults { - protected final Writable[] values; + protected final HbaseObjectWritable[] values; protected int size; protected int count; public ParallelResults(int size) { - this.values = new Writable[size]; + this.values = new HbaseObjectWritable[size]; this.size = size; } @@ -757,9 +763,7 @@ public class HBaseClient { * @param conf configuration * @param factory socket factory */ - public HBaseClient(Class valueClass, Configuration conf, - SocketFactory factory) { - this.valueClass = valueClass; + public HBaseClient(Configuration conf, SocketFactory factory) { this.maxIdleTime = conf.getInt("hbase.ipc.client.connection.maxidletime", 10000); //10s this.maxConnectRetries = @@ -783,8 +787,8 @@ public class HBaseClient { * @param valueClass value class * @param conf configuration */ - public HBaseClient(Class valueClass, Configuration conf) { - this(valueClass, conf, NetUtils.getDefaultSocketFactory(conf)); + public HBaseClient(Configuration conf) { + this(conf, NetUtils.getDefaultSocketFactory(conf)); } /** Return the socket factory of this client @@ -830,12 +834,12 @@ public class HBaseClient { * @return Writable * @throws IOException e */ - public Writable call(Writable param, InetSocketAddress address, HBaseRPCOptions options) - throws IOException { + public HbaseObjectWritable call(Writable param, InetSocketAddress address, + HBaseRPCOptions options) throws IOException { return call(param, address, null, 0, options); } - public Writable call(Writable param, InetSocketAddress addr, + public HbaseObjectWritable call(Writable param, InetSocketAddress addr, UserGroupInformation ticket, int rpcTimeout, HBaseRPCOptions options) throws IOException { @@ -912,10 +916,10 @@ public class HBaseClient { * @return Writable[] * @throws IOException e */ - public Writable[] call(Writable[] params, InetSocketAddress[] addresses, + public HbaseObjectWritable[] call(Writable[] params, InetSocketAddress[] addresses, HBaseRPCOptions options) throws IOException { - if (addresses.length == 0) return new Writable[0]; + if (addresses.length == 0) return new HbaseObjectWritable[0]; ParallelResults results = new ParallelResults(params.length); // TODO this synchronization block doesnt make any sense, we should possibly fix it diff --git a/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java b/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java index 9739c39..efb1a0c 100644 --- a/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java +++ b/src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java @@ -205,7 +205,7 @@ public class HBaseRPC { HBaseClient client = clients.get(factory); if (client == null) { // Make an hbase client instead of hadoop Client. - client = new HBaseClient(HbaseObjectWritable.class, conf, factory); + client = new HBaseClient(conf, factory); clients.put(factory, client); } return client; @@ -273,9 +273,8 @@ public class HBaseRPC { if (isTraceEnabled) { startTime = System.currentTimeMillis(); } - HbaseObjectWritable value = (HbaseObjectWritable) - client.call(new Invocation(method, args), address, ticket, - rpcTimeout, options); + HbaseObjectWritable value = client.call(new Invocation(method, args), + address, ticket, rpcTimeout, options); if (isTraceEnabled) { long callTime = System.currentTimeMillis() - startTime; LOG.trace("Call: " + method.getName() + " " + callTime); -- 1.7.0.4