You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Ryan Rawson <ry...@gmail.com> on 2010/09/15 00:43:46 UTC

Review Request: HBASE-2941 reader threads for hbase rpc

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/
-----------------------------------------------------------

Review request for hbase.


Summary
-------

HBASE-2941 reader threads for hbase rpc


This addresses bug HBASE-2941.
    http://issues.apache.org/jira/browse/HBASE-2941


Diffs
-----

  src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 

Diff: http://review.cloudera.org/r/846/diff


Testing
-------


Thanks,

Ryan


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1213
-----------------------------------------------------------


Looks good.  Do the RPC tests pass?  There are a few small things below.


src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4124>

    What happens now if key is not acceptable?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4125>

    Now we let the IE out?



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4126>

    No need of the limit of ten anymore because pool is bounded?


- stack


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1216
-----------------------------------------------------------

Ship it!



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4128>

    do you not have to wait here until indication that the selector thread has woken up and is in the "wait" loop? I'm not 100% sure of the state preconditions for the NIO stuff


- Todd


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1219
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4132>

    This won't happen now?


- Andrew


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1217
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4131>

    this seems to be some kind of coordinate hand off so that the reader gets notified immediately of new things to read.  
    
    this is mostly a straightforward port of the hadoop bug so i cant answer to the suitability of the design really... but it does work... reliably.



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4129>

    Not really sure, i dont think it should be possible, since this thread will only have 'accept' bits set on the sockets... but this is just a straightforward port of the HADOOP bug.



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4130>

    they used to accept up to 10 connections, then read from 1, then go back into the select loop.  
    
    now the listener thread only accepts 1, then passes that to another thread to do the reads, so it doesnt have to attempt to keep socket accept() delays down by accepting batches at a time.


- Ryan


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1222
-----------------------------------------------------------

Ship it!


+1 from me too

- Todd


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1220
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<http://review.cloudera.org/r/846/#comment4134>

    The IE doesnt get thrown anymore, it was being thrown inside doRead() which we no longer call...


- Ryan


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: HBASE-2941 reader threads for hbase rpc

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/846/#review1221
-----------------------------------------------------------

Ship it!


- Andrew


On 2010-09-14 15:43:46, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/846/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 15:43:46)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2941 reader threads for hbase rpc
> 
> 
> This addresses bug HBASE-2941.
>     http://issues.apache.org/jira/browse/HBASE-2941
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java d3c6c21 
> 
> Diff: http://review.cloudera.org/r/846/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>