HBASE-14798 NPE reporting server load causes regionserver abort; causes 
TestAcidGuarantee to fail


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/43506320
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/43506320
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/43506320

Branch: refs/heads/hbase-12439
Commit: 43506320a1bb6ca2193162edfb5dee21fffc08a9
Parents: 1fa7b71
Author: stack <st...@apache.org>
Authored: Sat Nov 14 09:07:39 2015 -0800
Committer: stack <st...@apache.org>
Committed: Sat Nov 14 09:07:39 2015 -0800

----------------------------------------------------------------------
 .../regionserver/DefaultStoreFileManager.java   |  1 +
 .../hadoop/hbase/regionserver/HRegion.java      | 53 ++++++++++++--------
 2 files changed, 34 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/43506320/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
index c143c40..9f38b9e 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultStoreFileManager.java
@@ -70,6 +70,7 @@ class DefaultStoreFileManager implements StoreFileManager {
 
   @Override
   public final Collection<StoreFile> getStorefiles() {
+    // TODO: I can return a null list of StoreFiles? That'll mess up clients. 
St.Ack 20151111
     return storefiles;
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/43506320/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 459d56e..994270b 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -960,16 +960,26 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
     initializeStores(reporter, status);
   }
 
-  private void writeRegionOpenMarker(WAL wal, long openSeqId) throws 
IOException {
-    Map<byte[], List<Path>> storeFiles = new TreeMap<byte[], 
List<Path>>(Bytes.BYTES_COMPARATOR);
+  /**
+   * @return Map of StoreFiles by column family
+   */
+  private NavigableMap<byte[], List<Path>> getStoreFiles() {
+    NavigableMap<byte[], List<Path>> allStoreFiles =
+      new TreeMap<byte[], List<Path>>(Bytes.BYTES_COMPARATOR);
     for (Store store: getStores()) {
-      ArrayList<Path> storeFileNames = new ArrayList<Path>();
-      for (StoreFile storeFile: store.getStorefiles()) {
+      Collection<StoreFile> storeFiles = store.getStorefiles();
+      if (storeFiles == null) continue;
+      List<Path> storeFileNames = new ArrayList<Path>();
+      for (StoreFile storeFile: storeFiles) {
         storeFileNames.add(storeFile.getPath());
       }
-      storeFiles.put(store.getFamily().getName(), storeFileNames);
+      allStoreFiles.put(store.getFamily().getName(), storeFileNames);
     }
+    return allStoreFiles;
+  }
 
+  private void writeRegionOpenMarker(WAL wal, long openSeqId) throws 
IOException {
+    Map<byte[], List<Path>> storeFiles = getStoreFiles();
     RegionEventDescriptor regionOpenDesc = 
ProtobufUtil.toRegionEventDescriptor(
       RegionEventDescriptor.EventType.REGION_OPEN, getRegionInfo(), openSeqId,
       getRegionServerServices().getServerName(), storeFiles);
@@ -977,15 +987,7 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
   }
 
   private void writeRegionCloseMarker(WAL wal) throws IOException {
-    Map<byte[], List<Path>> storeFiles = new TreeMap<byte[], 
List<Path>>(Bytes.BYTES_COMPARATOR);
-    for (Store store: getStores()) {
-      ArrayList<Path> storeFileNames = new ArrayList<Path>();
-      for (StoreFile storeFile: store.getStorefiles()) {
-        storeFileNames.add(storeFile.getPath());
-      }
-      storeFiles.put(store.getFamily().getName(), storeFileNames);
-    }
-
+    Map<byte[], List<Path>> storeFiles = getStoreFiles();
     RegionEventDescriptor regionEventDesc = 
ProtobufUtil.toRegionEventDescriptor(
       RegionEventDescriptor.EventType.REGION_CLOSE, getRegionInfo(), 
mvcc.getReadPoint(),
       getRegionServerServices().getServerName(), storeFiles);
@@ -1016,7 +1018,9 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
       new HDFSBlocksDistribution();
     synchronized (this.stores) {
       for (Store store : this.stores.values()) {
-        for (StoreFile sf : store.getStorefiles()) {
+        Collection<StoreFile> storeFiles = store.getStorefiles();
+        if (storeFiles == null) continue;
+        for (StoreFile sf : storeFiles) {
           HDFSBlocksDistribution storeFileBlocksDistribution =
             sf.getHDFSBlockDistribution();
           hdfsBlocksDistribution.add(storeFileBlocksDistribution);
@@ -1059,7 +1063,6 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
     for (HColumnDescriptor family: tableDescriptor.getFamilies()) {
       Collection<StoreFileInfo> storeFiles = 
regionFs.getStoreFiles(family.getNameAsString());
       if (storeFiles == null) continue;
-
       for (StoreFileInfo storeFileInfo : storeFiles) {
         try {
           
hdfsBlocksDistribution.add(storeFileInfo.computeHDFSBlocksDistribution(fs));
@@ -1639,10 +1642,16 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
   public long getOldestHfileTs(boolean majorCompactioOnly) throws IOException {
     long result = Long.MAX_VALUE;
     for (Store store : getStores()) {
-      for (StoreFile file : store.getStorefiles()) {
-        HFile.Reader reader = file.getReader().getHFileReader();
+      Collection<StoreFile> storeFiles = store.getStorefiles();
+      if (storeFiles == null) continue;
+      for (StoreFile file : storeFiles) {
+        StoreFile.Reader sfReader = file.getReader();
+        if (sfReader == null) continue;
+        HFile.Reader reader = sfReader.getHFileReader();
+        if (reader == null) continue;
         if (majorCompactioOnly) {
           byte[] val = 
reader.loadFileInfo().get(StoreFile.MAJOR_COMPACTION_KEY);
+          if (val == null) continue;
           if (val == null || !Bytes.toBoolean(val)) {
             continue;
           }
@@ -4898,7 +4907,9 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
     if (LOG.isTraceEnabled()) {
       LOG.trace(getRegionInfo().getEncodedName() + " : Store files for region: 
");
       for (Store s : stores.values()) {
-        for (StoreFile sf : s.getStorefiles()) {
+        Collection<StoreFile> storeFiles = s.getStorefiles();
+        if (storeFiles == null) continue;
+        for (StoreFile sf : storeFiles) {
           LOG.trace(getRegionInfo().getEncodedName() + " : " + sf);
         }
       }
@@ -5006,7 +5017,9 @@ public class HRegion implements HeapSize, 
PropagatingConfigurationObserver, Regi
           throw new IllegalArgumentException("No column family : " +
               new String(column) + " available");
         }
-        for (StoreFile storeFile: store.getStorefiles()) {
+        Collection<StoreFile> storeFiles = store.getStorefiles();
+        if (storeFiles == null) continue;
+        for (StoreFile storeFile: storeFiles) {
           storeFileNames.add(storeFile.getPath().toString());
         }
 

Reply via email to