hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Appy (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-19047) CP exposed Scanner types should not extend Shipper
Date Fri, 27 Oct 2017 20:49:00 GMT

    [ https://issues.apache.org/jira/browse/HBASE-19047?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16222789#comment-16222789

Appy commented on HBASE-19047:

bq. This was done as part of another comment fix. This is rare chance that the post hook just
close the core scanner being passed to it and create a scanner using region#getScanner() and
return back. Even if this happens and we dont take that as shipper now, what happens, we can
not cal shipped after every RPC. Still there wont be a func issue. We will delay the ref count
decr on blocks until the scanner close happens. So I am ok to remove this extra 'shipper '

Your explanation gave me more context. Based on it, I feel like real issue here is wrong ownership.
If you want to keep the "shipper" around even if the original scanner is destroyed, than the
scanner, neither interface nor any implementation, should "own" it. It should probably be

> CP exposed Scanner types should not extend Shipper
> --------------------------------------------------
>                 Key: HBASE-19047
>                 URL: https://issues.apache.org/jira/browse/HBASE-19047
>             Project: HBase
>          Issue Type: Sub-task
>          Components: Coprocessors
>            Reporter: Anoop Sam John
>            Assignee: Anoop Sam John
>            Priority: Critical
>             Fix For: 2.0.0-alpha-4
>         Attachments: HBASE-19047.patch, HBASE-19047_V2.patch, HBASE-19047_V2.patch, HBASE-19047_V3.patch,
HBASE-19047_V4.patch, HBASE-19047_V4.patch, HBASE-19047_V4.patch
> Shipper is a IA.Private interface and very much internal.. 
> Right now CP exposed RegionScanner is extending this and so exposing the shipped() method.
This by mistake is called, can harm the correctness of the cells in the Results.
> preScannerOpen() allowing to return a new Scanner is also problematic now.  This can
allow users to create a Region scanner from Region and then wrap it and return back (Well
same can be done by postScannerOpen also), it can so happen that the wrapper is not implementing
the shipped() properly.  In any way exposing the shipped () is problematic.
> Solution Steps
> 1. Remove preScannerOpen() , the use case I can think of is wrapping the original scanner.
The original scanner can be created by Region.getScanner way only..  May be no need to remove
this hook.  Just remove the ability for it to return a RegionScanner instance.  Call this
with the Scan object and the CP can change the Scan object if they want.
> 2. Let RegionScanner not extending Shipper but only RegionScannerImpl implements this
> 3. We have ref to the RegionScanner created by core and let that be used by RegionScannerShippedCallBack
when the post hook doing a wrap.

This message was sent by Atlassian JIRA

View raw message