On Fri, Sep 29, 2017 at 1:18 PM, Umesh Agashe <[email protected]> wrote:
> Hi, > > Currently Region.processRowsWithLocks() API takes > o.a.h.h.regionserver.RowProcessor as an argument and only implementation > of > this class is MultiRowMutationProcessor. This implementation is internal > and used from HRegion.mutateRows...() methods. > > HRegion.processRowsWithLocks() implementation, doesn't call coprocessor > hooks but instead calls RowProcessor hooks at appropriate point in > execution. Many of these hooks/ methods have same names and are called at > similar points during the course of execution but they are not related! > > Confusing! > HRegion.batchMutate() methods call coprocessor hooks but not row > RowProcessor hooks. > > Doubly confounding! > Internal implementation MultiRowMutationProcessor, call coprocessor hooks > from inside it's own methods/ hooks. But this can not be expected of all > implementations for RowProcessors. > > > In case of HRegion.batchMutate...() methods, CP mutations are merged with > input mutations and these merged mutations are applied to WALEdit fetched > from CPs. > > In case of processRowsWithLocks(), mutations are fetched from RowProcessor > instance and are applied on WALEdit built by RowProcessor. > > The major inconsistency here is, one code path uses coprocessors while > other uses RowProcessor. There are other minor inconsistencies along those > two code paths. > > RowProcessor seems to have arrived after Coprocessor. commit ce36877d30efb21a6c62a72c47cf51b442576fda Author: Zhihong Yu <[email protected]> Date: Wed Mar 21 18:25:18 2012 +0000 HBASE-5542 Unify HRegion.mutateRowsWithLocks() and HRegion.processRow() (Scott Chen) Coprocessor main classes show up here: commit 7299a72715f0ef1b05e478bee2e60d8f26fc2c24 Author: Andrew Kyle Purtell <[email protected]> Date: Sat Nov 20 01:23:39 2010 +0000 HBASE-2001 Coprocessors: Colocate user code with regions Maybe the overlap was not considered back then? > Proposed fix: > > * Unify two code paths. > +1 > * Deprecate RowProcessor and API Region.processRowsWithLocks() that takes > RowProcessor as an argument. > * Provide alternate API that doesn't take RowProcessor. > * Modify batchMutate...() to take additional arguments: rowsToLock > (byte[][]) and atomic/ allOrNone (boolean). > * Remove MultiRowMutationProcessor. Make HRegion.mutateRows() methods to > use batchMutate(). > * Make new implementation of Region.processRowsWithLocks() which doesn't > take RowProcessor as an argument use batchMutate(). > > Suggestion is that coprocessors can be used to do things RowProcessors are > doing. > > Seems right to me. Speak up if you have a reason for why RowProcessors should stick around. I see MultiRowMutationEndpoint for doing atomic x-row mutations on meta but is implemented otherwise. Otherwise, looking at HBASE-5542 linked issues, we have all the facility referenced implemented other than via RowProcessor (I think -- would need to dig more). Thanks Umesh, St.Ack > Related JIRAs: HBASE-18703, HBASE-18183 > > Let me know your thoughts. > > Thanks, > Umesh >
