See HBASE-18747. I'm currently working on it. I think this can be done by wrapping an InternalScanner.
2017-10-11 9:55 GMT+08:00 Anoop John <anoop.hb...@gmail.com>: > 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.hb...@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 <apurt...@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 <ghelml...@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.hb...@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 < > apurt...@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.hb...@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 < > apurt...@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 >