hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Purtell <andrew.purt...@gmail.com>
Subject Re: [DISCUSS] deprecating o.a.h.h.regionserver.RowProcessor
Date Sat, 30 Sep 2017 21:05:14 GMT
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 <stack@duboce.net> wrote:
> 
>> On Fri, Sep 29, 2017 at 1:18 PM, Umesh Agashe <uagashe@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 <tedyu@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 <apurtell@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
>> 

Mime
View raw message