You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ra...@apache.org on 2012/01/21 18:23:53 UTC
svn commit: r1234372 - in /hbase/branches/0.90: ./
src/main/java/org/apache/hadoop/hbase/regionserver/
src/main/java/org/apache/hadoop/hbase/regionserver/wal/
src/test/java/org/apache/hadoop/hbase/regionserver/wal/
Author: ramkrishna
Date: Sat Jan 21 17:23:52 2012
New Revision: 1234372
URL: http://svn.apache.org/viewvc?rev=1234372&view=rev
Log:
HBASE-5225 Backport HBASE-3845 -data loss because lastSeqWritten can miss memstore edits (Ram)
Modified:
hbase/branches/0.90/CHANGES.txt
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java
hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java
Modified: hbase/branches/0.90/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/CHANGES.txt?rev=1234372&r1=1234371&r2=1234372&view=diff
==============================================================================
--- hbase/branches/0.90/CHANGES.txt (original)
+++ hbase/branches/0.90/CHANGES.txt Sat Jan 21 17:23:52 2012
@@ -35,6 +35,7 @@ Release 0.90.6 - Unreleased
leading to recreation of regions that were deleted (Ram)
HBASE-5153 Add retry logic in HConnectionImplementation#resetZooKeeperTrackers (Jieshan)
HBASE-5196 Failure in region split after PONR could cause region hole (Jimmy Xiang)
+ HBASE-5225 Backport HBASE-3845 -data loss because lastSeqWritten can miss memstore edits (Ram)
IMPROVEMENT
HBASE-5102 Change the default value of the property "hbase.connection.per.config" to false in
Modified: hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1234372&r1=1234371&r2=1234372&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sat Jan 21 17:23:52 2012
@@ -948,7 +948,8 @@ public class HRegion implements HeapSize
final long currentMemStoreSize = this.memstoreSize.get();
List<StoreFlusher> storeFlushers = new ArrayList<StoreFlusher>(stores.size());
try {
- sequenceId = (wal == null)? myseqid: wal.startCacheFlush();
+ sequenceId = (wal == null) ? myseqid : wal
+ .startCacheFlush(this.regionInfo.getEncodedNameAsBytes());
completeSequenceId = this.getCompleteCacheFlushSequenceId(sequenceId);
for (Store s : stores.values()) {
@@ -997,7 +998,9 @@ public class HRegion implements HeapSize
// We used to only catch IOEs but its possible that we'd get other
// exceptions -- e.g. HBASE-659 was about an NPE -- so now we catch
// all and sundry.
- if (wal != null) wal.abortCacheFlush();
+ if (wal != null) {
+ wal.abortCacheFlush(this.regionInfo.getEncodedNameAsBytes());
+ }
DroppedSnapshotException dse = new DroppedSnapshotException("region: " +
Bytes.toStringBinary(getRegionName()));
dse.initCause(t);
Modified: hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java?rev=1234372&r1=1234371&r2=1234372&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java Sat Jan 21 17:23:52 2012
@@ -662,6 +662,20 @@ public class HLog implements Syncable {
}
}
+ private byte[] getSnapshotName(byte[] encodedRegionName) {
+ byte snp[] = new byte[encodedRegionName.length + 3];
+ // an encoded region name has only hex digits. s, n or p are not hex
+ // and therefore snapshot-names will never collide with
+ // encoded-region-names
+ snp[0] = 's';
+ snp[1] = 'n';
+ snp[2] = 'p';
+ for (int i = 0; i < encodedRegionName.length; i++) {
+ snp[i + 3] = encodedRegionName[i];
+ }
+ return snp;
+ }
+
/*
* Clean up old commit logs.
* @return If lots of logs, flush the returned region so next time through
@@ -1224,18 +1238,52 @@ public class HLog implements Syncable {
/**
* By acquiring a log sequence ID, we can allow log messages to continue while
* we flush the cache.
- *
+ *
* Acquire a lock so that we do not roll the log between the start and
* completion of a cache-flush. Otherwise the log-seq-id for the flush will
* not appear in the correct logfile.
- *
- * @return sequence ID to pass {@link #completeCacheFlush(byte[], byte[], long, boolean)}
- * (byte[], byte[], long)}
+ *
+ * Ensuring that flushes and log-rolls don't happen concurrently also allows
+ * us to temporarily put a log-seq-number in lastSeqWritten against the region
+ * being flushed that might not be the earliest in-memory log-seq-number for
+ * that region. By the time the flush is completed or aborted and before the
+ * cacheFlushLock is released it is ensured that lastSeqWritten again has the
+ * oldest in-memory edit's log-sequence-number for the region that was being
+ * flushed.
+ *
+ * In this method, by removing the entry in lastSeqWritten for the region
+ * being flushed we ensure that the next edit inserted in this region will be
+ * correctly recorded in {@link #append(HRegionInfo, HLogKey, WALEdit)}. The
+ * of the earliest in-memory log-sequence-number - which is now in the
+ * memstore snapshot - is saved temporarily in the lastSeqWritten map while
+ * the flush is active.
+ *
+ * @return sequence ID to pass
+ * {@link #completeCacheFlush(byte[], byte[], long, boolean)} (byte[],
+ * byte[], long)}
* @see #completeCacheFlush(byte[], byte[], long, boolean)
* @see #abortCacheFlush()
*/
- public long startCacheFlush() {
+ public long startCacheFlush(final byte[] encodedRegionName) {
this.cacheFlushLock.lock();
+ Long seq = this.lastSeqWritten.remove(encodedRegionName);
+ // seq is the log-sequence-number of the oldest edit associated with this
+ // region. If a snapshot already exists - because the last flush failed -
+ // then seq will
+ // be the log-sequence-number of the oldest edit in the snapshot
+ if (seq != null) {
+ // keeping the earliest sequence number of the snapshot in
+ // lastSeqWritten maintains the correctness of
+ // getOldestOutstandingSeqNum(). But it doesn't matter really because
+ // everything is being done inside of cacheFlush lock.
+ Long oldseq = lastSeqWritten.put(getSnapshotName(encodedRegionName), seq);
+ if (oldseq != null) {
+ LOG.error("Logic Error Snapshot seq id from earlier flush still"
+ + " present! for region " + Bytes.toString(encodedRegionName)
+ + " overwritten oldseq=" + oldseq + "with new seq=" + seq);
+ Runtime.getRuntime().halt(1);
+ }
+ }
return obtainSeqNum();
}
@@ -1265,15 +1313,15 @@ public class HLog implements Syncable {
writeTime += System.currentTimeMillis() - now;
writeOps++;
this.numEntries.incrementAndGet();
- Long seq = this.lastSeqWritten.get(encodedRegionName);
- if (seq != null && logSeqId >= seq.longValue()) {
- this.lastSeqWritten.remove(encodedRegionName);
- }
}
// 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));
this.cacheFlushLock.unlock();
}
}
@@ -1292,7 +1340,25 @@ public class HLog implements Syncable {
* currently is a restart of the regionserver so the snapshot content dropped
* by the failure gets restored to the memstore.
*/
- public void abortCacheFlush() {
+ public void abortCacheFlush(byte[] encodedRegionName) {
+ Long snapshot_seq = this.lastSeqWritten
+ .remove(getSnapshotName(encodedRegionName));
+ if (snapshot_seq != null) {
+ // updateLock not necessary because we are racing against
+ // lastSeqWritten.putIfAbsent() in append() and we will always win,
+ // before releasing cacheFlushLock make sure that the region's entry in
+ // lastSeqWritten points to the earliest edit in the region
+ Long current_memstore_earliest_seq = this.lastSeqWritten.put(
+ encodedRegionName, snapshot_seq);
+ if (current_memstore_earliest_seq != null
+ && (current_memstore_earliest_seq.longValue() <= snapshot_seq
+ .longValue())) {
+ LOG.error("Logic Error region " + Bytes.toString(encodedRegionName)
+ + " acquired edits out of order current memstore seq="
+ + current_memstore_earliest_seq + " snapshot seq=" + snapshot_seq);
+ Runtime.getRuntime().halt(1);
+ }
+ }
this.cacheFlushLock.unlock();
}
Modified: hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java?rev=1234372&r1=1234371&r2=1234372&view=diff
==============================================================================
--- hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java (original)
+++ hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java Sat Jan 21 17:23:52 2012
@@ -457,7 +457,7 @@ public class TestHLog {
HRegionInfo info = new HRegionInfo(new HTableDescriptor(tableName),
row,Bytes.toBytes(Bytes.toString(row) + "1"), false);
log.append(info, tableName, cols, System.currentTimeMillis());
- long logSeqId = log.startCacheFlush();
+ long logSeqId = log.startCacheFlush(info.getEncodedNameAsBytes());
log.completeCacheFlush(info.getEncodedNameAsBytes(), tableName, logSeqId, info.isMetaRegion());
log.close();
Path filename = log.computeFilename();
@@ -525,7 +525,7 @@ public class TestHLog {
HRegionInfo hri = new HRegionInfo(new HTableDescriptor(tableName),
HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW);
log.append(hri, tableName, cols, System.currentTimeMillis());
- long logSeqId = log.startCacheFlush();
+ long logSeqId = log.startCacheFlush(hri.getEncodedNameAsBytes());
log.completeCacheFlush(hri.getEncodedNameAsBytes(), tableName, logSeqId, false);
log.close();
Path filename = log.computeFilename();
@@ -633,7 +633,7 @@ public class TestHLog {
// Flush the first region, we expect to see the first two files getting
// archived
- long seqId = log.startCacheFlush();
+ long seqId = log.startCacheFlush(hri.getEncodedNameAsBytes());
log.completeCacheFlush(hri.getEncodedNameAsBytes(), tableName, seqId, false);
log.rollWriter();
assertEquals(2, log.getNumLogFiles());
@@ -641,7 +641,7 @@ public class TestHLog {
// Flush the second region, which removes all the remaining output files
// since the oldest was completely flushed and the two others only contain
// flush information
- seqId = log.startCacheFlush();
+ seqId = log.startCacheFlush(hri2.getEncodedNameAsBytes());
log.completeCacheFlush(hri2.getEncodedNameAsBytes(), tableName2, seqId, false);
log.rollWriter();
assertEquals(0, log.getNumLogFiles());
Modified: hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java?rev=1234372&r1=1234371&r2=1234372&view=diff
==============================================================================
--- hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java (original)
+++ hbase/branches/0.90/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java Sat Jan 21 17:23:52 2012
@@ -356,7 +356,7 @@ public class TestWALReplay {
}
// Add a cache flush, shouldn't have any effect
- long logSeqId = wal.startCacheFlush();
+ long logSeqId = wal.startCacheFlush(regionName);
wal.completeCacheFlush(regionName, tableName, logSeqId, hri.isMetaRegion());
// Add an edit to another family, should be skipped.