[
https://issues.apache.org/jira/browse/HBASE-19047?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16222728#comment-16222728
]
Anoop Sam John commented on HBASE-19047:
----------------------------------------
Sorry Stack for not making it clear. I was saying abt the below comment from
Appy.
{quote}
If we are trying to hide it, then we shouldn't be doing returnedScanner
instanceOf Shipper and reassigning shipper.
If we are not trying to hide it, then what we have right now is just fine.
{quote}
That is abt this code part in patch
{code}
RegionScannerImpl coreScanner = region.getScanner(scan);
+ Shipper shipper = coreScanner;
+ RegionScanner scanner = coreScanner;
if (region.getCoprocessorHost() != null) {
scanner = region.getCoprocessorHost().postScannerOpen(scan, scanner);
+ if (scanner instanceof Shipper) {
+ shipper = (Shipper) scanner;
+ }
}
{code}
After post hook call, we check whether the shipper assign to be changed with
what the post hook returned. Now as we dont expose Shipper to CPs now, what is
the need of this is his Q. Ya it make sense. 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 ' assignment.
> 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
(v6.4.14#64029)
