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

Jurriaan Mous edited comment on HBASE-15921 at 6/1/16 8:42 PM:
---------------------------------------------------------------

{quote}
This has to be this way? (Lots of abstract classes...)
public abstract class AsyncRegionServerCallable<T> extends 
AbstractRegionServerCallable<T>
It probably has to be given you are coming into a convoluted hierarchy that has 
accreted over a long period of time. Was just wondering if could have a 
shallower hierarchy. No issue if can't be done easily... or has to wait till 
later after you've gotten your async client in.
Or, you just moved this existing class out to its own file?
{quote}

I have to add the abstract classes right now to share code between the sync and 
async variants. As usage of the sync variants are replaced with async variants 
we can remove the abstract versions again.

bq. AsyncTable returns Future only? Not CompletableFuture? Consumers won't be 
able to consume AsyncTable in an event-driven way? We need a callback?

This is indeed as [~Apache9] mentions the Netty and now also HBase variant with 
event driven methods.

bq. Why we let out EventExecutor? Especially given it a netty class. Can we 
contain the fact that the implementation is betty-based?

At the moment I use the event executors for (new/failed/succes) promise 
creation. I can take a fresh look at it to see if I can contain it more but am 
not sure if it is possible. 

bq. In successful future it has a class comment "25     * A Failed Response 
future"

Will fix it.

bq. Any chance of a note on how the PromiseKeepers work?

Yes I will. It is the async way to combine multiple calls. So it replaces the 
batch callback in places.

bq. The replacement of HTable by TableImpl comes later? 

It is already replaced at getTable in Connection in this patch. I want to make 
sure the performance is ok and then I find a solution for the remaining HTable 
refs.



Created review: https://reviews.apache.org/r/48152/


was (Author: jurmous):
{quote}
This has to be this way? (Lots of abstract classes...)
public abstract class AsyncRegionServerCallable<T> extends 
AbstractRegionServerCallable<T>
It probably has to be given you are coming into a convoluted hierarchy that has 
accreted over a long period of time. Was just wondering if could have a 
shallower hierarchy. No issue if can't be done easily... or has to wait till 
later after you've gotten your async client in.
Or, you just moved this existing class out to its own file?
{quote}

I have to add the abstract classes right now to share code between the sync and 
async variants. As usage of the sync variants are replaced with async variants 
we can remove the abstract versions again.

bq. AsyncTable returns Future only? Not CompletableFuture? Consumers won't be 
able to consume AsyncTable in an event-driven way? We need a callback?

This is indeed as [~Apache9] mentions the Netty variant with event driven 
methods.

bq. Why we let out EventExecutor? Especially given it a netty class. Can we 
contain the fact that the implementation is betty-based?

At the moment I use the event executors for (new/failed/succes) promise 
creation. I can take a fresh look at it to see if I can contain it more but am 
not sure if it is possible. 

bq. In successful future it has a class comment "25     * A Failed Response 
future"

Will fix it.

bq. Any chance of a note on how the PromiseKeepers work?

Yes I will. It is the async way to combine multiple calls. So it replaces the 
batch callback in places.

bq. The replacement of HTable by TableImpl comes later? 

It is already replaced at getTable in Connection in this patch. I want to make 
sure the performance is ok and then I find a solution for the remaining HTable 
refs.



Created review: https://reviews.apache.org/r/48152/

> Add first AsyncTable impl and create TableImpl based on it
> ----------------------------------------------------------
>
>                 Key: HBASE-15921
>                 URL: https://issues.apache.org/jira/browse/HBASE-15921
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Jurriaan Mous
>            Assignee: Jurriaan Mous
>         Attachments: HBASE-15921.patch, HBASE-15921.v1.patch
>
>
> First we create an AsyncTable interface with implementation without the Scan 
> functionality. Those will land in a separate patch since they need a refactor 
> of existing scans.
> Also added is a new TableImpl to replace HTable. It uses the AsyncTableImpl 
> internally and should be a bit faster because it does jump through less hoops 
> to do ProtoBuf transportation. This way we can run all existing tests on the 
> AsyncTableImpl to guarantee its quality.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to