I'm +1 on remove the general bypass support, but I think we can still provide the bypass logic for some operations if we can give a clear and stable semantic of what will be bypassed. Need to know some usages and review the methods case by case.
Thanks. 2017-10-11 6:30 GMT+08:00 Andrew Purtell <apurt...@apache.org>: > > 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 >
