From 5dc9e51723871dfa03a3a43b909907050e20c80e Mon Sep 17 00:00:00 2001 From: stack Date: Thu, 12 Nov 2015 15:01:52 -1000 Subject: [PATCH] Fix npe, get rid of some dup'ed code. Add other npe checks. --- .../regionserver/DefaultStoreFileManager.java | 1 + .../apache/hadoop/hbase/regionserver/HRegion.java | 53 ++++++++++++++-------- 2 files changed, 34 insertions(+), 20 deletions(-) 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 getStorefiles() { + // TODO: I can return a null list of StoreFiles? That'll mess up clients. St.Ack 20151111 return storefiles; } 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> storeFiles = new TreeMap>(Bytes.BYTES_COMPARATOR); + /** + * @return Map of StoreFiles by column family + */ + private NavigableMap> getStoreFiles() { + NavigableMap> allStoreFiles = + new TreeMap>(Bytes.BYTES_COMPARATOR); for (Store store: getStores()) { - ArrayList storeFileNames = new ArrayList(); - for (StoreFile storeFile: store.getStorefiles()) { + Collection storeFiles = store.getStorefiles(); + if (storeFiles == null) continue; + List storeFileNames = new ArrayList(); + 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> 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> storeFiles = new TreeMap>(Bytes.BYTES_COMPARATOR); - for (Store store: getStores()) { - ArrayList storeFileNames = new ArrayList(); - for (StoreFile storeFile: store.getStorefiles()) { - storeFileNames.add(storeFile.getPath()); - } - storeFiles.put(store.getFamily().getName(), storeFileNames); - } - + Map> 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 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 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 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 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 storeFiles = store.getStorefiles(); + if (storeFiles == null) continue; + for (StoreFile storeFile: storeFiles) { storeFileNames.add(storeFile.getPath().toString()); } -- 2.6.1