From a51dc02ed3b7252add956dd81f712c7f7053d823 Mon Sep 17 00:00:00 2001 From: Elliott Clark Date: Fri, 18 Dec 2015 14:14:25 -0800 Subject: [PATCH] HBASE-15014 Fix filterCellByStore in WALsplitter is awful for performance --- .../hbase/regionserver/wal/WALEdit.java | 14 ++++++++++++- .../apache/hadoop/hbase/wal/WALSplitter.java | 20 ++++++++++++------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java index 5e53e411243..c47ce13b307 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java @@ -99,7 +99,7 @@ public class WALEdit implements Writable, HeapSize { private final int VERSION_2 = -1; private final boolean isReplay; - private final ArrayList cells = new ArrayList(1); + private ArrayList cells = new ArrayList(1); public static final WALEdit EMPTY_WALEDIT = new WALEdit(); @@ -170,6 +170,18 @@ public class WALEdit implements Writable, HeapSize { return cells; } + /** + * This is not thread safe. + * This will change the WALEdit and shouldn't be used unless you are sure that nothing + * else depends on the contents being immutable. + * + * @param cells the list of cells that this WALEdit now contains. + */ + @InterfaceAudience.Private + public void setCells(ArrayList cells) { + this.cells = cells; + } + public NavigableMap getAndRemoveScopes() { NavigableMap result = scopes; scopes = null; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java index 98882ff1051..c047f8d6600 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java @@ -1513,21 +1513,27 @@ public class WALSplitter { if (maxSeqIdInStores == null || maxSeqIdInStores.isEmpty()) { return; } - List skippedCells = new ArrayList(); + // Create the array list for the cells that aren't filtered. + // We make the assumption that most cells will be kept. + ArrayList keptCells = new ArrayList(logEntry.getEdit().getCells().size()); for (Cell cell : logEntry.getEdit().getCells()) { - if (!CellUtil.matchingFamily(cell, WALEdit.METAFAMILY)) { + if (CellUtil.matchingFamily(cell, WALEdit.METAFAMILY)) { + keptCells.add(cell); + } else { byte[] family = CellUtil.cloneFamily(cell); Long maxSeqId = maxSeqIdInStores.get(family); // Do not skip cell even if maxSeqId is null. Maybe we are in a rolling upgrade, // or the master was crashed before and we can not get the information. - if (maxSeqId != null && maxSeqId.longValue() >= logEntry.getKey().getLogSeqNum()) { - skippedCells.add(cell); + if (maxSeqId == null || maxSeqId.longValue() < logEntry.getKey().getLogSeqNum()) { + keptCells.add(cell); } } } - if (!skippedCells.isEmpty()) { - logEntry.getEdit().getCells().removeAll(skippedCells); - } + + // Anything in the keptCells array list is still live. + // So rather than removing the cells from the array list + // which would be an O(n^2) operation, we just replace the list + logEntry.getEdit().setCells(keptCells); } @Override