diff --git src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 3bd75b8..a2d4661 100644
--- src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -711,8 +711,8 @@ public class HRegion implements HeapSize { // , Writable{
wasFlushing = writestate.flushing;
LOG.debug("Closing " + this + ": disabling compactions & flushes");
while (writestate.compacting > 0 || writestate.flushing) {
- LOG.debug("waiting for " + writestate.compacting + " compactions" +
- (writestate.flushing ? " & cache flush" : "") +
+ LOG.debug("Waiting for " + writestate.compacting + " compaction(s)" +
+ (writestate.flushing ? " & a cache flush" : "") +
" to complete for region " + this);
try {
writestate.wait();
@@ -1147,7 +1147,7 @@ public class HRegion implements HeapSize { // , Writable{
LOG.debug("Started memstore flush for " + this +
", current region memstore size " +
StringUtils.humanReadableInt(this.memstoreSize.get()) +
- ((wal != null)? "": "; wal is null, using passed sequenceid=" + myseqid));
+ ((wal != null)? "": "; wal is null, using passed seqid=" + myseqid));
}
// Stop updates while we snapshot the memstore of all stores. We only have
diff --git src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
index 623edbe..cfd4c9a 100644
--- src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
+++ src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
@@ -758,7 +758,7 @@ public class HLog implements Syncable {
/*
* @return Logs older than this id are safe to remove.
*/
- private Long getOldestOutstandingSeqNum() {
+ Long getOldestOutstandingSeqNum() {
return Collections.min(this.lastSeqWritten.values());
}
@@ -1323,7 +1323,6 @@ public class HLog implements Syncable {
return obtainSeqNum();
}
-
/**
* Complete the cache flush
*
@@ -1353,17 +1352,11 @@ public class HLog implements Syncable {
}
// sync txn to file system
this.sync();
-
} finally {
// updateLock not needed for removing snapshot's entry
// Cleaning up of lastSeqWritten is in the finally clause because we
// don't want to confuse getOldestOutstandingSeqNum()
- this.lastSeqWritten.remove(getSnapshotName(encodedRegionName));
- Long l = this.lastSeqWritten.remove(encodedRegionName);
- if (l != null) {
- LOG.warn("Why is there a raw encodedRegionName in lastSeqWritten? name=" +
- Bytes.toString(encodedRegionName) + ", seqid=" + l);
- }
+ Long l = this.lastSeqWritten.remove(getSnapshotName(encodedRegionName));
this.cacheFlushLock.unlock();
}
}
diff --git src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java
index 16f0ee6..09aaf96 100644
--- src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java
+++ src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java
@@ -20,8 +20,9 @@
package org.apache.hadoop.hbase.regionserver.wal;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.lang.reflect.Method;
@@ -38,19 +39,23 @@ import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hbase.*;
-import org.apache.hadoop.hbase.regionserver.wal.HLog.Reader;
-import org.apache.hadoop.hbase.util.Bytes;
-import org.apache.hadoop.hbase.util.FSUtils;
import org.apache.hadoop.hbase.Coprocessor;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HColumnDescriptor;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
import org.apache.hadoop.hbase.coprocessor.SampleRegionWALObserver;
+import org.apache.hadoop.hbase.regionserver.wal.HLog.Reader;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.FSUtils;
import org.apache.hadoop.hdfs.DFSClient;
import org.apache.hadoop.hdfs.DistributedFileSystem;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.hdfs.protocol.FSConstants;
import org.apache.hadoop.hdfs.server.datanode.DataNode;
-import org.apache.hadoop.hdfs.server.namenode.FSNamesystem;
import org.apache.hadoop.hdfs.server.namenode.LeaseManager;
import org.apache.hadoop.io.SequenceFile;
import org.apache.log4j.Level;
@@ -122,12 +127,64 @@ public class TestHLog {
oldLogDir = new Path(hbaseDir, ".oldlogs");
dir = new Path(hbaseDir, getName());
}
+
private static String getName() {
// TODO Auto-generated method stub
return "TestHLog";
}
/**
+ * @see HBASE-4789 does overzealous pruning of seqids
+ * @throws IOException
+ */
+ @Test
+ public void testRemoveLastSequenceWrittenOnClose() throws IOException {
+ final byte [] tableName = Bytes.toBytes("testRemoveLastSequenceWrittenOnClose");
+ HLog log = new HLog(fs, dir, oldLogDir, conf);
+ HRegionInfo hri = new HRegionInfo(tableName,
+ HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW);
+
+ // Add a single edit. Assert oldest seqid makes sense and that its
+ // what we see when we start a flush as the associated flush seqid.
+ addEdits(log, hri, tableName, 1);
+ long oldest = log.getOldestOutstandingSeqNum();
+ long seqid = log.getSequenceNumber();
+ assertEquals(oldest, seqid);
+
+ // Flush the region and in between start and complete of flush add more edits
+ long currentseqid = startCacheFlush(log, hri, oldest);
+ addEdits(log, hri, tableName, 1);
+ assertEquals(oldest, log.getOldestOutstandingSeqNum().longValue());
+ // Complete of flush should remove oldest.
+ log.completeCacheFlush(hri.getEncodedNameAsBytes(), tableName, currentseqid, false);
+ long newoldest = log.getOldestOutstandingSeqNum().longValue();
+ assertTrue(oldest < newoldest);
+
+ // OK. Now act like we're doing a close. During close no edits come in
+ // between flush start and end and we should therefore be letting go of
+ // oldest edit for the flushing region.
+ oldest = newoldest;
+ currentseqid = startCacheFlush(log, hri, oldest);
+ log.completeCacheFlush(hri.getEncodedNameAsBytes(), tableName, currentseqid, false);
+ assertNull(log.getOldestOutstandingSeqNum());
+ }
+
+ /**
+ * Start a cache flush and make sure of some assertions while at it.
+ * @param log
+ * @param hri
+ * @param oldest
+ * @return currentseqid
+ */
+ private long startCacheFlush(final HLog log, final HRegionInfo hri,
+ final long oldest) {
+ long currentseqid = log.startCacheFlush(hri.getEncodedNameAsBytes());
+ assertEquals(oldest, log.getOldestOutstandingSeqNum().longValue());
+ assertEquals(oldest + 1, currentseqid);
+ return currentseqid;
+ }
+
+ /**
* Just write multiple logs then split. Before fix for HADOOP-2283, this
* would fail.
* @throws IOException
@@ -173,7 +230,7 @@ public class TestHLog {
}
log.close();
HLogSplitter logSplitter = HLogSplitter.createLogSplitter(conf,
- hbaseDir, logdir, this.oldLogDir, this.fs);
+ hbaseDir, logdir, TestHLog.oldLogDir, TestHLog.fs);
List splits =
logSplitter.splitLog();
verifySplits(splits, howmany);
@@ -434,7 +491,7 @@ public class TestHLog {
// Make sure you can read all the content
SequenceFile.Reader reader
- = new SequenceFile.Reader(this.fs, walPath, this.conf);
+ = new SequenceFile.Reader(TestHLog.fs, walPath, TestHLog.conf);
int count = 0;
HLogKey key = HLog.newKey(conf);
WALEdit val = new WALEdit();
@@ -682,8 +739,8 @@ public class TestHLog {
assertNotNull(c);
}
- private void addEdits(HLog log, HRegionInfo hri, byte [] tableName,
- int times) throws IOException {
+ private void addEdits(HLog log, HRegionInfo hri, byte [] tableName, int times)
+ throws IOException {
HTableDescriptor htd = new HTableDescriptor();
htd.addFamily(new HColumnDescriptor("row"));