hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-2425) Crossport HADOOP-1849 rpc fix
Date Fri, 04 Nov 2011 18:13:51 GMT

    [ https://issues.apache.org/jira/browse/HBASE-2425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144228#comment-13144228
] 

jiraposter@reviews.apache.org commented on HBASE-2425:
------------------------------------------------------



bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > Just a couple of comments.  Otherwise looks good to me.

Excellent.  Thanks for the review.


bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java, line 322
bq.  > <https://reviews.apache.org/r/2718/diff/1/?file=56222#file56222line322>
bq.  >
bq.  >     We could eliminate the flag and use status instead.  Are there plans for other
bits being set in this?  Otherwise, we always have length and error can be determined from
Status.  Or would removing this break asynchbase in other ways?

Its for asynchbase -- so it can tell diff between a response with length and one w/o (its
trying to support all hbase versions).


bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java, line 98
bq.  > <https://reviews.apache.org/r/2718/diff/1/?file=56223#file56223line98>
bq.  >
bq.  >     Don't think this is necessary?  The super.readFields(in) should throw VersionMismatchException
if the read version doesn't match our getVersion().

My mistake.  Will fix.


bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java, line 116
bq.  > <https://reviews.apache.org/r/2718/diff/1/?file=56223#file56223line116>
bq.  >
bq.  >     Probably better to use super.write(out) here.  Same code, but future proof to
changes.

Ditto.


bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java, line 60
bq.  > <https://reviews.apache.org/r/2718/diff/1/?file=56227#file56227line60>
bq.  >
bq.  >     Should be able to remove this now that Invocation implements VersionedWritable.

Agreed.


bq.  On 2011-11-04 18:06:38, Gary Helmling wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java, line 343
bq.  > <https://reviews.apache.org/r/2718/diff/1/?file=56227#file56227line343>
bq.  >
bq.  >     Should be able to remove this now that Invocation implements VersionedWritable.
 I didn't see any dependency on rpc version outside of the Invocation serialization.

Good.


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2718/#review3049
-----------------------------------------------------------


On 2011-11-04 00:11:21, Michael Stack wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2718/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-11-04 00:11:21)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Versions of Gary suggestions
bq.  
bq.  
bq.  This addresses bug hbase-2425.
bq.      https://issues.apache.org/jira/browse/hbase-2425
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateImplementation.java fce5490

bq.    src/main/java/org/apache/hadoop/hbase/coprocessor/AggregateProtocol.java 2fa4d6f 
bq.    src/main/java/org/apache/hadoop/hbase/coprocessor/BaseEndpointCoprocessor.java 6f88357

bq.    src/main/java/org/apache/hadoop/hbase/ipc/CoprocessorProtocol.java 6fcb771 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 1365411 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 4a8918a 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java e60f970 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/ProtocolSignature.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/Status.java PRE-CREATION 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/VersionedProtocol.java fb07374 
bq.    src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java 60a9248 
bq.    src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java 8de2314 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java 0d0e4c5 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 12bd33e 
bq.    src/test/java/org/apache/hadoop/hbase/ipc/TestDelayedRpc.java 888f428 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/TestServerCustomProtocol.java e5b6a78

bq.  
bq.  Diff: https://reviews.apache.org/r/2718/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Michael
bq.  
bq.


                
> Crossport HADOOP-1849 rpc fix
> -----------------------------
>
>                 Key: HBASE-2425
>                 URL: https://issues.apache.org/jira/browse/HBASE-2425
>             Project: HBase
>          Issue Type: Task
>            Reporter: stack
>              Labels: moved_from_0_20_5
>
> Suggested over in HBASE-2360.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message