[jira] [Commented] (HBASE-8355) BaseRegionObserver#preCompactScannerOpen returns null

["Jesse Yates (JIRA)" <ji...@apache.org>]

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

Jesse Yates commented on HBASE-8355:
------------------------------------

Looking at this more, I think Lar's comment doesn't apply:
{quote}
I don't think this is right. The internal scanner passed is not the right scanner to use. I am surprised that this does not break many more tests.

In Compactor.compact the result of preCompactScannerOpen need to be null in order to have the default action happening.
{quote}

The code that call this is:
{code}
  public InternalScanner preCompactScannerOpen(Store store, List<StoreFileScanner> scanners,
      ScanType scanType, long earliestPutTs, CompactionRequest request) throws IOException {
    ObserverContext<RegionCoprocessorEnvironment> ctx = null;
    InternalScanner s = null;
    for (RegionEnvironment env: coprocessors) {
      if (env.getInstance() instanceof RegionObserver) {
        ctx = ObserverContext.createAndPrepare(env, ctx);
        try {
          s = ((RegionObserver) env.getInstance()).preCompactScannerOpen(ctx, store, scanners,
            scanType, earliestPutTs, s, request);
        } catch (Throwable e) {
          handleCoprocessorThrowable(env,e);
        }
        if (ctx.shouldComplete()) {
          break;
        }
      }
    }
    return s;
  }
{code}

Which starts by passing in null as the InternalScanner and pulls an internal scanner back out. So returning the passed InternalScanner - s - should be the correct default. 

So back to +1 for Andy's original, unless I'm missing something?

This led to two other things (that shouldn't necessarily be tackled here):
1. This same logic is present in preFlushScannerOpen and preCompactScanner open, with the same consequences
2. There is no easy solution for nested CPs creating new scanners - creating a brand new scanner will ignore any previous and setting Context#complete will ignore later CPs. I'm working on a first cut solution and will open a new jira (or at least blog about it :) when I figure out what it looks like... unless someone has a suggestions?
                
> BaseRegionObserver#preCompactScannerOpen returns null
> -----------------------------------------------------
>
>                 Key: HBASE-8355
>                 URL: https://issues.apache.org/jira/browse/HBASE-8355
>             Project: HBase
>          Issue Type: Bug
>          Components: Coprocessors
>    Affects Versions: 0.98.0, 0.94.8, 0.95.1
>            Reporter: Andrew Purtell
>            Assignee: Andrew Purtell
>         Attachments: 8355-0.94.patch, 8355.patch
>
>
> As pointed out in https://github.com/forcedotcom/phoenix/pull/131, BaseRegionObserver#preCompactScannerOpen returns null by default, which hoses any coprocessors down the line, making override of this method mandatory. The fix is trivial, patch coming momentarily.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira