HBase
  1. HBase
  2. HBASE-5532

get NPE during MajorCompactionChecker

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: regionserver
    • Labels:
      None

      Description

      We found error log (NullPointerException) below on our online cluster:

      2012-03-05 00:17:09,592 ERROR org.apache.hadoop.hbase.regionserver.HRegionServer$MajorCompactionChecker: Caught exception
      java.lang.NullPointerException
      at org.apache.hadoop.hbase.regionserver.Store.isMajorCompaction(Store.java:878)
      at org.apache.hadoop.hbase.regionserver.Store.isMajorCompaction(Store.java:857)
      at org.apache.hadoop.hbase.regionserver.HRegion.isMajorCompaction(HRegion.java:3017)
      at org.apache.hadoop.hbase.regionserver.HRegionServer$MajorCompactionChecker.chore(HRegionServer.java:1172)
      at org.apache.hadoop.hbase.Chore.run(Chore.java:66)

      After Check the code we found although it already check whether store files has null reader at the begin of the function(isMajorCompaction), but it still has some possibility the reader is closed before it return(eg mini compaction). So we need to check store file reader before we use it to avoid this NPE

      1. HBASE-5532-v2.patch
        1 kB
        terry zhang
      2. HBASE-5532.patch
        0.7 kB
        terry zhang

        Activity

        terry zhang created issue -
        terry zhang made changes -
        Field Original Value New Value
        Attachment HBASE-5532.patch [ 12517211 ]
        Hide
        Ted Yu added a comment -

        Which version of HBase were you using ?

        Show
        Ted Yu added a comment - Which version of HBase were you using ?
        Hide
        terry zhang added a comment -

        Hi, Ted. Our is base on 0.90.2 (including many patches from 90.3 till 90.5).

        Show
        terry zhang added a comment - Hi, Ted. Our is base on 0.90.2 (including many patches from 90.3 till 90.5).
        Hide
        Nicolas Spiegelberg added a comment -

        Sounds like this is a race condition, so this patch wouldn't be sufficient. We need some way to lock the reader during this query.

        Show
        Nicolas Spiegelberg added a comment - Sounds like this is a race condition, so this patch wouldn't be sufficient. We need some way to lock the reader during this query.
        Hide
        terry zhang added a comment -

        yes,Nicolas. This is a race condition and we can use lock to avoid this issue. And now in the function toDetermines if Store should be split also didn't use lock to protect.

        Store.java
          /**
           * Determines if Store should be split
           * @return byte[] if store should be split, null otherwise.
           */
          public byte[] getSplitPoint() {
            this.lock.readLock().lock();
            try {
              // sanity checks
              if (this.storefiles.isEmpty()) {
                return null;
              }
         
        
         
        
                StoreFile.Reader r = sf.getReader(); * <= check if the reader is null*
                if (r == null) {
                  LOG.warn("Storefile " + sf + " Reader is null");
                  continue;
                }
        
                long size = r.length();
                if (size > maxSize) {
                  // This is the largest one so far
                  maxSize = size;
                  largestSf = sf;
                }
              }
        
              StoreFile.Reader r = largestSf.getReader(); *<= check if the reader is null*
              if (r == null) {
                LOG.warn("Storefile " + largestSf + " Reader is null");
                return null;
              }
              // Get first, last, and mid keys.  Midkey is the key that starts block
              // in middle of hfile.  Has column and timestamp.  Need to return just
              // the row we want to split on as midkey.
              byte [] midkey = r.midkey();
             .....
            return null;
          }
        
        Show
        terry zhang added a comment - yes,Nicolas. This is a race condition and we can use lock to avoid this issue. And now in the function toDetermines if Store should be split also didn't use lock to protect. Store.java /** * Determines if Store should be split * @ return byte [] if store should be split, null otherwise. */ public byte [] getSplitPoint() { this .lock.readLock().lock(); try { // sanity checks if ( this .storefiles.isEmpty()) { return null ; } StoreFile.Reader r = sf.getReader(); * <= check if the reader is null * if (r == null ) { LOG.warn( "Storefile " + sf + " Reader is null " ); continue ; } long size = r.length(); if (size > maxSize) { // This is the largest one so far maxSize = size; largestSf = sf; } } StoreFile.Reader r = largestSf.getReader(); *<= check if the reader is null * if (r == null ) { LOG.warn( "Storefile " + largestSf + " Reader is null " ); return null ; } // Get first, last, and mid keys. Midkey is the key that starts block // in middle of hfile. Has column and timestamp. Need to return just // the row we want to split on as midkey. byte [] midkey = r.midkey(); ..... return null ; }
        Hide
        terry zhang added a comment -

        add a read lock during isMajorCompaction() to avoid race condition.

        Show
        terry zhang added a comment - add a read lock during isMajorCompaction() to avoid race condition.
        terry zhang made changes -
        Attachment HBASE-5532-v2.patch [ 12517521 ]
        Hide
        stack added a comment -

        This looks like patch for 0.90 only?

        Show
        stack added a comment - This looks like patch for 0.90 only?

          People

          • Assignee:
            Unassigned
            Reporter:
            terry zhang
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development