> On 2010-06-16 21:24:18, stack wrote:
> > src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java, line 283
> > <http://review.hbase.org/r/191/diff/1/?file=1423#file1423line283>
> >
> >     I presume we go to use this heap later in this method? If so, could it 
> > be null then?  Can another method be clearing this.heap outside of a 
> > synchronization?
> 
> Ryan Rawson wrote:
>     All the synchronized entry points are protected against null heaps, 
> except this one.  When the heap is null, they will re-seek the scanners, 
> which involves a lot of work.  What happens in prod is that the seek will 
> kick in, but since there is no thread local in ReadWriteConcurrency it will 
> NPE at that point, thus causing the stack trace in the JIRA.
>     
>     So when we get a updateReaders() and we have already null'ed out the 
> heap, just do nothing.  The heap will be null only right after an 
> updateReaders() and before any next()/peek().

Ok.  Can you add a comment to the same effect as above on commit.  After 
reading above I have better idea of what going on.


> On 2010-06-16 21:24:18, stack wrote:
> > src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java, line 
> > 469
> > <http://review.hbase.org/r/191/diff/1/?file=1424#file1424line469>
> >
> >     Why does this test for the NPE?  Why all the gratuitous changes above 
> > this change?
> 
> Ryan Rawson wrote:
>     the other lines are trailing whitespace changes, my editor kills them out 
> of any file it saves.
>     
>     This tests for NPE because we call updateReaders() twice in a row w/o 
> calling next() - the particular pattern that was happening in the prod 
> server.  In this case removing the other fix causes an NPE b/c this.store is 
> null.

Please add comment to test.  Helps.


- stack


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/191/#review248
-----------------------------------------------------------


On 2010-06-16 17:22:31, Ryan Rawson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/191/
> -----------------------------------------------------------
> 
> (Updated 2010-06-16 17:22:31)
> 
> 
> Review request for hbase and stack.
> 
> 
> Summary
> -------
> 
> HBASE-2740  NPE in ReadWriteConsistencyControl
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java 6d4546f 
>   src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java 76ab7b5 
> 
> Diff: http://review.hbase.org/r/191/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryan
> 
>

Reply via email to