You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by li...@apache.org on 2014/04/23 20:18:23 UTC
svn commit: r1589480 - in /hbase/branches/0.89-fb/src:
main/java/org/apache/hadoop/hbase/regionserver/
test/java/org/apache/hadoop/hbase/regionserver/
Author: liyin
Date: Wed Apr 23 18:18:22 2014
New Revision: 1589480
URL: http://svn.apache.org/r1589480
Log:
[master] Fix negative sequenceid exception
Author: everyoung
Summary:
Fix negative sequenceid exception in HRegionSeqidTransanction when opening region with no families.
The HLog sequence id always starts from 1, so using 0 to indicate an empty record is fine.
Test Plan: All tests
Reviewers: daviddeng, jiqingt, aaiyer, rshroff, liyintang, elliott
Reviewed By: daviddeng
CC: hbase-dev@
Differential Revision: https://phabricator.fb.com/D1289443
Task ID: 4192110
Modified:
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionSeqidTransition.java
hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSeqidTransition.java
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1589480&r1=1589479&r2=1589480&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Wed Apr 23 18:18:22 2014
@@ -3307,9 +3307,9 @@ public class HRegion implements HeapSize
// add seqid to hlog
// to be strict, next sequence id is getSequenceNumber() + 1
// because of incrementAndGet in Hlog.java obtainSeqNum method
- // after calling writeToHLog method nex line.
+ // after calling log.writeSeqidTransition next line.
HRegionSeqidTransition seqidTransition =
- new HRegionSeqidTransition(seqid-1, log.getSequenceNumber() + 1);
+ new HRegionSeqidTransition(Math.max(seqid-1, 0), log.getSequenceNumber() + 1);
// no serverInfo b/c outside an HRS context
log.writeSeqidTransition(seqidTransition, null, r.getRegionInfo());
}
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionSeqidTransition.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionSeqidTransition.java?rev=1589480&r1=1589479&r2=1589480&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionSeqidTransition.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionSeqidTransition.java Wed Apr 23 18:18:22 2014
@@ -35,12 +35,12 @@ public final class HRegionSeqidTransitio
private final long nextSeqid;
public HRegionSeqidTransition(final long lastSeqid, final long nextSeqid) {
- if ((lastSeqid >= 0) && (nextSeqid >= 0)) {
- this.lastSeqid = lastSeqid;
- this.nextSeqid = nextSeqid;
- } else {
+ if (lastSeqid < 0 || nextSeqid < 0) {
throw new IllegalArgumentException("Seqids cannot be negative!");
}
+
+ this.lastSeqid = lastSeqid;
+ this.nextSeqid = nextSeqid;
}
public long getLastSeqid() {
@@ -60,18 +60,19 @@ public final class HRegionSeqidTransitio
if (fromByte == null) {
return null;
}
- if (fromByte.length >= 3 * Bytes.SIZEOF_LONG) {
- if (Bytes.toLong(fromByte, 0) == VERSION) {
- return new HRegionSeqidTransition(
- Bytes.toLong(fromByte, Bytes.SIZEOF_LONG),
- Bytes.toLong(fromByte, 2 * Bytes.SIZEOF_LONG));
- } else {
- // can be changed to deal versions later
- throw new IllegalArgumentException("Only version 1 exists!!");
- }
- } else {
+
+ if (fromByte.length < 3 * Bytes.SIZEOF_LONG) {
throw new IllegalArgumentException("Bytes array too short!");
}
+
+ if (Bytes.toLong(fromByte, 0) != VERSION) {
+ // can be changed to deal versions later
+ throw new IllegalArgumentException("Only version 1 exists!!");
+ }
+
+ return new HRegionSeqidTransition(
+ Bytes.toLong(fromByte, Bytes.SIZEOF_LONG),
+ Bytes.toLong(fromByte, 2 * Bytes.SIZEOF_LONG));
}
/**
Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1589480&r1=1589479&r2=1589480&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Wed Apr 23 18:18:22 2014
@@ -2545,8 +2545,11 @@ public class HRegionServer implements HR
// If hlog.seqid = 100, seqid = 200, after hlog.setSequenceNumber(200),
// hlog.getSequenceNumber may return 210 if there're 10 edits inbound.
// The transition seqid is still valid
+ // to be strict, next sequence id is getSequenceNumber() + 1
+ // because of incrementAndGet in Hlog.java obtainSeqNum method
+ // after calling hlog.writeSeqidTransition method next line.
HRegionSeqidTransition seqidTransition =
- new HRegionSeqidTransition(seqid - 1, hlog.getSequenceNumber());
+ new HRegionSeqidTransition(Math.max(seqid-1, 0), hlog.getSequenceNumber() + 1);
LOG.info("Sequence id of region " + regionInfo.getRegionNameAsString() +
" has a transition from " + seqidTransition.getLastSeqid() +
" to " + seqidTransition.getNextSeqid() +
Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSeqidTransition.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSeqidTransition.java?rev=1589480&r1=1589479&r2=1589480&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSeqidTransition.java (original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionSeqidTransition.java Wed Apr 23 18:18:22 2014
@@ -31,9 +31,12 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseTestingUtility;
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.MasterNotRunningException;
import org.apache.hadoop.hbase.MiniHBaseCluster;
+import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.HTable;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.ResultScanner;
@@ -103,6 +106,23 @@ public class TestHRegionSeqidTransition
}
}
+ @Test
+ public void testNegSeqid() throws IOException {
+ HTableDescriptor htd = new HTableDescriptor(TABLENAME + "NegSeqid");
+ HRegionInfo hri = new HRegionInfo(htd, null, null);
+ HRegion.createHRegion(hri, TEST_UTIL.getTestDir(), c);
+ HLog log = new HLog(TEST_UTIL.getTestFileSystem(),
+ new Path(TEST_UTIL.getTestDir(), HConstants.HREGION_LOGDIR_NAME),
+ new Path(TEST_UTIL.getTestDir(), HConstants.HREGION_OLDLOGDIR_NAME),
+ c, null);
+ try {
+ HRegion.openHRegion(hri, TEST_UTIL.getTestDir(), log, c);
+ } catch (IllegalArgumentException e) {
+ assertTrue("Got negative seqid exception when opening region with no families!.", false);
+ }
+ HRegion.deleteRegion(TEST_UTIL.getTestFileSystem(), TEST_UTIL.getTestDir(), hri);
+ }
+
@Test(timeout=300000)
public void testSeqidTransitionOnRegionShutdown()
throws Exception {