hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anoop John <anoop.hb...@gmail.com>
Subject Re: [DISCUSSION] Removing the bypass semantic from the Coprocessor APIs
Date Wed, 11 Oct 2017 01:55:00 GMT
The discussion abt the pre hook return a not null object came in one
of the CP cleanup issue.  We were discussing whether we should expose
APIs for the StoreScanner object creation.  As long as this pre hook
return way and so bypass the actual core code,  we need to expose.
Duo raised this concern that time..  WDYT  Duo Zhang?

-Anoop-

On Wed, Oct 11, 2017 at 7:23 AM, Anoop John <anoop.hbase@gmail.com> wrote:
>>The Tephra TransactionProcessor CP makes use of bypass() in preDelete() to
> This is interesting code to read.     There is a way for doing this
> with out bypass.  Make use of the hook preBatchMutate().   This hook
> takes MiniBatchOperationInProgress to which we can add some Mutations
> generated from CPs.    From Deletes, the Puts can be created and added
> ( #addOperationsFromCP) .   Also mark the corresponding Delete
> mutations as done using #setOperationStatus(int index, OperationStatus
> opStatus)  with OperationStatus#SUCCESS..    The core code part will
> ignore these Delete mutations then and process those puts, been added
> by CPs.
>
> Just a suggestion on how the op can be done with out using the bypass
> way.   But the current way is looking simpler. The bypass was of help.
>
> -Anoop-
>
> On Wed, Oct 11, 2017 at 4:00 AM, Andrew Purtell <apurtell@apache.org> wrote:
>>> The Tephra TransactionProcessor CP makes use of bypass() in preDelete()
>> to override handling of delete tombstones in a transactional way
>>
>> Hmm. This is an interesting case. I can't think of how Deletes could be
>> converted into Puts from this code, from when the handling of the Delete is
>> already in progress, but it could be possible to add another hook somewhere
>> in RPC dispatch ahead of when we have moved through RsRpcServices down into
>> HRegion, and replace the incoming Delete with Puts there without a need to
>> bypass.
>>
>>> The CDAP IncrementHandler CP also makes use of bypass() in preGetOp() and
>> preIncrementAfterRRowLock() to provide a transaction implementation of
>> readless increments
>>
>> This one might be possible to achieve using the wrapped scanner and a
>> replacement of the Get object handed down into core code with something
>> that is useless and harmless rather than a bypass.
>>
>> These are great examples.
>>
>>
>> On Tue, Oct 10, 2017 at 12:19 PM, Gary Helmling <ghelmling@gmail.com> wrote:
>>
>>> The Tephra TransactionProcessor CP makes use of bypass() in preDelete() to
>>> override handling of delete tombstones in a transactional way:
>>> https://github.com/apache/incubator-tephra/blob/master/
>>> tephra-hbase-compat-1.3/src/main/java/org/apache/tephra/hbase/coprocessor/
>>> TransactionProcessor.java#L244
>>>
>>> The CDAP IncrementHandler CP also makes use of bypass() in preGetOp() and
>>> preIncrementAfterRRowLock() to provide a transaction implementation of
>>> readless increments:
>>> https://github.com/caskdata/cdap/blob/develop/cdap-hbase-
>>> compat-1.1/src/main/java/co/cask/cdap/data2/increment/
>>> hbase11/IncrementHandler.java#L121
>>>
>>> What would be the alternate approach for these applications?  In both cases
>>> they need to impose their own semantics on the underlying KeyValue
>>> storage.  Is there a different way this can be done?
>>>
>>>
>>> On Tue, Oct 10, 2017 at 11:58 AM Anoop John <anoop.hbase@gmail.com> wrote:
>>>
>>> > Wrap core scanners is different right?  That can be done in post
>>> > hooks.  I have seen many use cases for this..  Its the question abt
>>> > the pre hooks where we have not yet created the core object (like
>>> > scanner).  The CP pre code itself doing the work of object creation
>>> > and so the core code is been bypassed.    Well the wrapping thing can
>>> > be done in pre hook also. First create the core object by CP code
>>> > itself and then do the wrapped object and return.. I have seen in one
>>> > jira issue where the usage was this way..   The wrapping can be done
>>> > in post also in such cases I believe.
>>> >
>>> > -Anoop-
>>> >
>>> > On Wed, Oct 11, 2017 at 12:23 AM, Andrew Purtell <apurtell@apache.org>
>>> > wrote:
>>> > > I think we should continue to support overriding function by object
>>> > > inheritance. I didn't mention this and am not proposing more than
>>> > removing
>>> > > the bypass() sematic. No more no less. Phoenix absolutely depends on
>>> > being
>>> > > able to wrap core scanners and return the wrappers.
>>> > >
>>> > >
>>> > > On Tue, Oct 10, 2017 at 11:50 AM, Anoop John <anoop.hbase@gmail.com>
>>> > wrote:
>>> > >
>>> > >> When we say bypass the core code, it can be done today not only
by
>>> > >> calling bypass but by returning a not null object for some of the
pre
>>> > >> hooks.  Like preScannerOpen() if it return a scanner object, we
will
>>> > >> avoid the remaining core code execution for creation of the
>>> > >> scanner(s).  So this proposal include this aspect also and remove
any
>>> > >> possible way of bypassing the core code by the CP hook code execution
>>> > >> ?   Am +1.
>>> > >>
>>> > >> -Anoop-
>>> > >>
>>> > >> On Tue, Oct 10, 2017 at 11:40 PM, Andrew Purtell <apurtell@apache.org
>>> >
>>> > >> wrote:
>>> > >> > The coprocessor API provides an environment method, bypass(),
that
>>> > when
>>> > >> > called from a preXXX hook will cause the core code to skip
all
>>> > remaining
>>> > >> > processing. This capability was introduced on HBASE-3348.
Since this
>>> > >> time I
>>> > >> > think we are more enlightened about the complications of this
>>> feature.
>>> > >> (Or,
>>> > >> > anyway, speaking for myself:)
>>> > >> >
>>> > >> > Not all hooks provide the bypass semantic. Where this is the
case
>>> the
>>> > >> > javadoc for the hook says so, but it can be missed. If you
call
>>> > bypass()
>>> > >> in
>>> > >> > a hook where it is not supported it is a no-op. This can lead
to a
>>> > poor
>>> > >> > developer experience.
>>> > >> >
>>> > >> > Where bypass is supported what is being bypassed is all of
the core
>>> > code
>>> > >> > implementing the remainder of the operation. In order to understand
>>> > what
>>> > >> > calling bypass() will skip, a coprocessor implementer should
read
>>> and
>>> > >> > understand all of the remaining code and its nuances. Although
I
>>> think
>>> > >> this
>>> > >> > is good practice for coprocessor developers in general, it
demands a
>>> > >> lot. I
>>> > >> > think it would provide a much better developer experience
if we
>>> didn't
>>> > >> > allow bypass, even though it means - in theory - a coprocessor
would
>>> > be a
>>> > >> > lot more limited in some ways than before. What is skipped
is
>>> > extremely
>>> > >> > version dependent. That core code will vary, perhaps significantly,
>>> > even
>>> > >> > between point releases. We do not provide the promise of consistent
>>> > >> > behavior even between point releases for the bypass semantic.
To
>>> > achieve
>>> > >> > that we could not change any code between hook points. Therefore
the
>>> > >> > coprocessor implementer becomes an HBase core developer in
practice
>>> as
>>> > >> soon
>>> > >> > as they rely on bypass(). Every release of HBase may break
the
>>> > assumption
>>> > >> > that the replacement for the bypassed code takes care of all
>>> necessary
>>> > >> > skipped concerns. Because those concerns can change at any
point,
>>> > such an
>>> > >> > assumption is never safe.
>>> > >> >
>>> > >> > I say "in theory" because I would be surprised if anyone is
relying
>>> on
>>> > >> the
>>> > >> > bypass for the above reason. I seem to recall that Phoenix
might use
>>> > it
>>> > >> in
>>> > >> > one place to promote a normal mutation into an atomic operation,
by
>>> > >> > substituting one for the other, but if so that objective could
be
>>> > >> > reimplemented using their new locking manager.
>>> > >> >
>>> > >> > --
>>> > >> > Best regards,
>>> > >> > Andrew
>>> > >>
>>> > >
>>> > >
>>> > >
>>> > > --
>>> > > Best regards,
>>> > > Andrew
>>> > >
>>> > > Words like orphans lost among the crosstalk, meaning torn from truth's
>>> > > decrepit hands
>>> > >    - A23, Crosstalk
>>> >
>>>
>>
>>
>>
>> --
>> Best regards,
>> Andrew
>>
>> Words like orphans lost among the crosstalk, meaning torn from truth's
>> decrepit hands
>>    - A23, Crosstalk

Mime
View raw message