HBASE-2740 NPE in ReadWriteConsistencyControl

Review Request #191 - Created June 16, 2010 and submitted

Ryan Rawson
old-hbase
hbase
stack
HBASE-2740  NPE in ReadWriteConsistencyControl


  1. 
      
  2. 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?
    1. 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().
    2. 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.
  3. Why does this test for the NPE?  Why all the gratuitous changes above this change?
    1. 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.
    2. Please add comment to test.  Helps.
  4. 
      
  1. +1 on commit as long as commit includes some version of explaination just given here on review board.  Does it have to go into trunk and branch?
    1. This patch is still not committed (along w/ accomodation of my criticisms made here in review.hbase.org).
  2. 
      
Loading...