Looks like it. Two arrivals on parallel tracks. I never noticed this. I guess I’ve never really looked at RowProcessor. I’m glad we are going to clean this up.
> On Sep 30, 2017, at 1:59 PM, Stack <st...@duboce.net> wrote: > >> On Fri, Sep 29, 2017 at 1:18 PM, Umesh Agashe <uaga...@cloudera.com> 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 <te...@apache.org> > 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 <apurt...@apache.org> > 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 >>
