svn commit: r1584854 - in /hbase/branches/0.89-fb/src: main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java

[li...@apache.org]

Author: liyin
Date: Fri Apr  4 18:18:35 2014
New Revision: 1584854

URL: http://svn.apache.org/r1584854
Log:
[HBASE-9704] Handle Call object properly on serverside

Author: adela

Summary:
Initially we implemented this to work only for profiling but there is one more usage
and that is quota remaining when doing scan on serverside. So now
- if client doesn't want profiling he is not sending call object (at least we are still saving on network here)
- if profiling is not required we anyway create call object on serverside
Also fixed the assertion on testResultLimits since it was completely ignored when I was running the test locally

Test Plan:
ran
- TestFromClientSide.testResultLimits
- TestHeaderSendReceive
- TestPerRequestProfiling

Reviewers: manukranthk, gauravm, fan

Reviewed By: manukranthk

CC: hbase-eng@

Differential Revision: https://phabricator.fb.com/D1257200

Task ID: 4065193

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java?rev=1584854&r1=1584853&r2=1584854&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java Fri Apr  4 18:18:35 2014
@@ -293,6 +293,7 @@ public abstract class HBaseServer {
      */
     public Call(HBaseRPCOptions options) {
       this.shouldProfile = options.getRequestProfiling();
+      this.profilingData = null;
     }
 
     @ThriftField(1)

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java?rev=1584854&r1=1584853&r2=1584854&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/ipc/thrift/ThriftCallStatsReporter.java Fri Apr  4 18:18:35 2014
@@ -229,19 +229,15 @@ public class ThriftCallStatsReporter ext
     if (useHeaderProtocol) {
       if (ctx.requestContext instanceof NiftyRequestContext) {
         Call call = getCallInfoFromClient(ctx.requestContext);
+        // we anyway want to set the call object on serverside
+        // since it is not used only for profiling
+        if (call == null) {
+          call = new Call(HBaseRPCOptions.DEFAULT);
+        }
         if (HRegionServer.enableServerSideProfilingForAllCalls.get()
             || (call != null && call.isShouldProfile())) {
-          // it is possible that call is null - if profiling is enabled only on
-          // serverside
-          if (call == null) {
-            call = new Call(HBaseRPCOptions.DEFAULT);
-          }
           call.setShouldProfile(true);
           call.setProfilingData(new ProfilingData());
-        } else if (call != null) {
-          // call is not null but profiling is not enabled, so set profiling
-          // data to null
-          call.setProfilingData(null);
         }
         HRegionServer.callContext.set(call);
       } else {
@@ -268,7 +264,7 @@ public class ThriftCallStatsReporter ext
       try {
         Call call = HRegionServer.callContext.get();
         HRegionServer.callContext.remove();
-        if (call!= null && call.isShouldProfile()) {
+        if (call != null && call.isShouldProfile()) {
           sendCallInfoToClient(call, ctx.requestContext);
         }
       } catch (Exception e) {

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java?rev=1584854&r1=1584853&r2=1584854&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java Fri Apr  4 18:18:35 2014
@@ -780,8 +780,9 @@ public class TestFromClientSide {
     try {
       get3 = new Get(ROWS[2]);
       ht.get(get3);
-      assert(false);
+      assertTrue(false);
     } catch (IOException e) {
+      LOG.error("Expected result size is greater than limit", e);
     }
 
     // try with the multiple rows.
@@ -791,8 +792,9 @@ public class TestFromClientSide {
       list.add(get1);
       list.add(get2);
       ht.get(list);
-      assert(false);
+      assertTrue(false);
     } catch (IOException e) {
+      LOG.error("Expected result size is greater than limit", e);
     }
 
     // reset the max response size