[jira] [Comment Edited] (HBASE-15921) Add first AsyncTable impl and create TableImpl based on it

["Jurriaan Mous (JIRA)" <ji...@apache.org>]

    [ 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)