From 3f026cb83af07a13159936e19e96fb3cb9b61680 Mon Sep 17 00:00:00 2001 From: Elliott Clark Date: Thu, 10 Mar 2016 14:48:57 -0800 Subject: [PATCH] HBASE-15441 Fix WAL splitting when region has moved multiple times Summary: Currently WAL splitting is broken when a region has been opened multiple times in recent minutes. Region open and region close write event markers to the wal. These markers should have the sequence id in them. However it is currently getting 1. That means that if a region has moved multiple times in the last few mins then multiple split log workers will try and create the recovered edits file for sequence id 1. One of the workers will fail and on failing they will delete the recovered edits. Causing all split wal attempts to fail. We need to: # make sure that close get the correct sequence id for open. # Filter all region events from recovered edits It appears that the close event with a sequence id of one is coming from region warm up. This patch fixes that by making sure the close on warm up doesn't happen. Also splitting will ignore any of the events that are already in the logs. Test Plan: Unit tests pass Differential Revision: https://reviews.facebook.net/D55557 --- .../apache/hadoop/hbase/regionserver/HRegion.java | 10 +++++---- .../RegionReplicaReplicationEndpoint.java | 5 +++++ .../org/apache/hadoop/hbase/wal/WALSplitter.java | 24 ++++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) 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 c44145e..a17df8c 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 @@ -966,10 +966,13 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi private void initializeWarmup(final CancelableProgressable reporter) throws IOException { MonitoredTask status = TaskMonitor.get().createStatus("Initializing region " + this); - // Initialize all the HStores status.setStatus("Warming up all the Stores"); - initializeStores(reporter, status); + try { + initializeStores(reporter, status); + } finally { + status.markComplete("Done warming up."); + } } /** @@ -6427,9 +6430,8 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi fs = FileSystem.get(conf); } - HRegion r = HRegion.newHRegion(tableDir, wal, fs, conf, info, htd, rsServices); + HRegion r = HRegion.newHRegion(tableDir, wal, fs, conf, info, htd, null); r.initializeWarmup(reporter); - r.close(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/RegionReplicaReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/RegionReplicaReplicationEndpoint.java index de87ac3..da37cfa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/RegionReplicaReplicationEndpoint.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/RegionReplicaReplicationEndpoint.java @@ -367,6 +367,11 @@ public class RegionReplicaReplicationEndpoint extends HBaseReplicationEndpoint { } @Override + public boolean keepRegionEvents() { + return true; + } + + @Override public List finishWritingAndClose() throws IOException { finishWriting(true); return 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 ad5774f..8d78480 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 @@ -359,6 +359,11 @@ public class WALSplitter { editsSkipped++; continue; } + // Don't send Compaction/Close/Open region events to recovered edit type sinks. + if (entry.getEdit().isMetaEdit() && !outputSink.keepRegionEvents()) { + editsSkipped++; + continue; + } entryBuffers.appendEntry(entry); editsCount++; int moreWritersFromLastCheck = this.getNumOpenWriters() - numOpenedFilesLastCheck; @@ -1266,6 +1271,15 @@ public class WALSplitter { public boolean flush() throws IOException { return false; } + + /** + * Some WALEdit's contain only KV's for account on what happened to a region. + * Not all sinks will want to get those edits. + * + * @return Return true if this sink wants to get all WALEdit's regardless of if it's a region + * event. + */ + public abstract boolean keepRegionEvents(); } /** @@ -1609,6 +1623,11 @@ public class WALSplitter { } } + @Override + public boolean keepRegionEvents() { + return false; + } + /** * @return a map from encoded region ID to the number of edits written out for that region. */ @@ -2060,6 +2079,11 @@ public class WALSplitter { return false; } + @Override + public boolean keepRegionEvents() { + return true; + } + void addWriterError(Throwable t) { thrown.add(t); } -- 2.7.3