Review Board 1.7.22


HBASE-5705

Review Request #5714 - Created July 2, 2012 and updated

Devaraj Das
trunk
HBASE-5705
Reviewers
apurtell, jxiang, stack, tedyu, tsuna
hbase
Attaching a patch that has been tested some. Running through the full suite. Meanwhile, please have a look at the patch. I should note that this patch has been inspired by the corresponding work in Hadoop RPC.
Ran unit tests.
Total:
25
Open:
9
Resolved:
16
Dropped:
0
Status:
From:
Description From Last Updated Status
What happens when I cache a client? What am I saving on? How do they get cleared out? Maybe answer ... Michael Stack July 4, 2012, 2:32 p.m. Open
This is some old doug cutting coment from original rpc? I never understood what it was on about. Where are ... Michael Stack July 4, 2012, 2:32 p.m. Open
When can the rpc be pure pb? This is sending the old call.id first? When can this all be done ... Michael Stack July 4, 2012, 2:32 p.m. Open
We write twice? Once to a buffer, then onto the wire? Just to get length? Doesn't pb do length internal? Michael Stack July 4, 2012, 2:32 p.m. Open
Still Writables? Michael Stack July 4, 2012, 2:32 p.m. Open
Per invocation? When we move to pb, will we still need to do this version check? Michael Stack July 4, 2012, 2:32 p.m. Open
Either remove 'other' or add 'components' after 'level' Ted Yu July 25, 2012, 7:18 p.m. Open
The 'call' method has the signature as such so that WritableRpcEngine can be supported. We need to support the WritableRpcEngine ... Devaraj Das July 24, 2012, 10:12 p.m. Open
Will we need this cache when we go all pb? When we go all pb, we'll pass what instead of ... Michael Stack July 25, 2012, 9:15 p.m. Open
Review request changed
Updated (July 10, 2012, 1:03 a.m.)
Will wait to hear from other reviewers before I upload a patch to address Ted's minor comments. For now, this is just answering the questions, Ted asked.
Posted (July 24, 2012, 10:12 p.m.)
Will wait to hear from other reviewers before I upload a patch to address Ted's minor comments. For now, this is just answering the questions, Ted asked.
The 'call' method has the signature as such so that WritableRpcEngine can be supported. We need to support the WritableRpcEngine still (until all the protocols have been converted to PB).
I promise to fix this once the WritableRpcEngine is removed :-) 
For now I am reusing the code. It also helps to maintain patch consistency (for example, if a patch updated the log method, my patch would fail to apply).
Ignore the changes in the TestFSUtils across the patches. I had included it by mistake in the first patch. If you download the last patch I uploaded, you wouldn't see the test updates in the patch.
Posted (July 25, 2012, 9:15 p.m.)

   

  
Will we need this cache when we go all pb?  When we go all pb, we'll pass what instead of the HbaseObjectWritable class?  Nothing?
  1. This cache is actually independent of PB (note that this cache existed even in the pure WritableRpcEngine world). I need to look at in more detail what would happen when we go all PB, but yeah the relevant argument probably needn't be passed.
I don't understand what this comment is about (I said that in last review IIRC).  Its old comment from early days.  Whats it mean for you?  What pool is it referring to?
  1. As I said on jira, I removed the comment (and hopefully the class javadoc is not confusing).