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

ramkrishna.s.vasudevan commented on HBASE-6900:
-----------------------------------------------

@Lars
If really the heap is not null then checkReseek will return immediately.  
Attaching the checkReseek code incase you dont have eclipse with you.
{code}
  private boolean checkReseek() throws IOException {
    if (this.heap == null && this.lastTop != null) {
      resetScannerStack(this.lastTop);
      if (this.heap.peek() == null
          || store.comparator.compare(this.lastTop, this.heap.peek()) != 0) {
        LOG.debug("Storescanner.peek() is changed where before = "
            + this.lastTop.toString() + ",and after = " + this.heap.peek());
        this.lastTop = null;
        return true;
      }
      this.lastTop = null; // gone!
    }
    // else dont need to reseek
    return false;
  }
{code}
Again why i added !checkReseek() and heap!=null is because
thought heap is not null the lasttop could be null?

{code}
Could we just check for heap == null in the beginning of the method and return 
false if so?
{code}
Actually we can return false on seeing heap to be null, but am not sure whether 
it will be generic.  Because if the CP impl is such a way that i do a reseek 
and return of false implies that the given row key is not available then it 
gives a different meaning.  But whereas if i reset the heap and then do a 
reseek then am ensuring that i am reseeking to the required kv on the newly 
created heap.
Infact our internal usage of heap needs this behaviour.  If you feel that is 
not generic then am open to change this Lars.

Thanks a lot for your review.


                
> RegionScanner.reseek() creates NPE when a flush or compaction happens before 
> the reseek.
> ----------------------------------------------------------------------------------------
>
>                 Key: HBASE-6900
>                 URL: https://issues.apache.org/jira/browse/HBASE-6900
>             Project: HBase
>          Issue Type: Bug
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: ramkrishna.s.vasudevan
>         Attachments: HBASE-6900_1.patch, HBASE-6900.patch
>
>
> HBASE-5520 introduced reseek() on the RegionScanner.  
> Now when a scanner is created we have the StoreScanner heap.  After this if a 
> flush or compaction happens parallely all the StoreScannerObservers are 
> cleared so that whenever a new next() call happens we tend to recreate the 
> scanner based on the latest store files.
> The reseek() in StoreScanner expects the heap not to be null because always 
> reseek would be called from next()
> {code}
> public synchronized boolean reseek(KeyValue kv) throws IOException {
>     //Heap cannot be null, because this is only called from next() which
>     //guarantees that heap will never be null before this call.
>     if (explicitColumnQuery && lazySeekEnabledGlobally) {
>       return heap.requestSeek(kv, true, useRowColBloom);
>     } else {
>       return heap.reseek(kv);
>     }
>   }
> {code}
> Now when we call RegionScanner.reseek() directly using CPs we tend to get a 
> NPE.  In our case it happened when a major compaction was going on.  I will 
> also attach a testcase to show the problem.

--
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

Reply via email to