You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2016/10/14 18:27:25 UTC
hbase git commit: Revert "HBASE-16698 Performance issue: handlers
stuck waiting for CountDownLatch inside WALKey#getWriteEntry under high
writing workload" Premature commit. Revert while discussion still ongoing.
Repository: hbase
Updated Branches:
refs/heads/master f555b5be9 -> 13baf4d37
Revert "HBASE-16698 Performance issue: handlers stuck waiting for CountDownLatch inside WALKey#getWriteEntry under high writing workload"
Premature commit. Revert while discussion still ongoing.
This reverts commit 9b13514483991889cd6ebe097c3c8eb0e7983e6d.
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/13baf4d3
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/13baf4d3
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/13baf4d3
Branch: refs/heads/master
Commit: 13baf4d37a7d3b4b0194dc616c8ac15959efa18f
Parents: f555b5b
Author: Michael Stack <st...@apache.org>
Authored: Fri Oct 14 11:27:05 2016 -0700
Committer: Michael Stack <st...@apache.org>
Committed: Fri Oct 14 11:27:05 2016 -0700
----------------------------------------------------------------------
.../hadoop/hbase/regionserver/HRegion.java | 85 +++++---------------
.../hbase/regionserver/wal/FSWALEntry.java | 19 ++---
.../org/apache/hadoop/hbase/wal/WALKey.java | 26 ------
.../hadoop/hbase/regionserver/TestHRegion.java | 7 +-
4 files changed, 29 insertions(+), 108 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/13baf4d3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
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 916aa98..ca92f06 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
@@ -64,7 +64,6 @@ import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.LongAdder;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.commons.logging.Log;
@@ -198,10 +197,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
public static final String LOAD_CFS_ON_DEMAND_CONFIG_KEY =
"hbase.hregion.scan.loadColumnFamiliesOnDemand";
- /** Config key for using mvcc pre-assign feature for put */
- public static final String HREGION_MVCC_PRE_ASSIGN = "hbase.hregion.mvcc.preassign";
- public static final boolean DEFAULT_HREGION_MVCC_PRE_ASSIGN = true;
-
/**
* This is the global default value for durability. All tables/mutations not
* defining a durability or using USE_DEFAULT will default to this value.
@@ -590,9 +585,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
// that has non-default scope
private final NavigableMap<byte[], Integer> replicationScope = new TreeMap<byte[], Integer>(
Bytes.BYTES_COMPARATOR);
- // flag and lock for MVCC preassign
- private final boolean mvccPreAssign;
- private final ReentrantLock preAssignMvccLock;
/**
* HRegion constructor. This constructor should only be used for testing and
@@ -752,14 +744,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
false :
conf.getBoolean(HConstants.ENABLE_CLIENT_BACKPRESSURE,
HConstants.DEFAULT_ENABLE_CLIENT_BACKPRESSURE);
-
- // get mvcc pre-assign flag and lock
- this.mvccPreAssign = conf.getBoolean(HREGION_MVCC_PRE_ASSIGN, DEFAULT_HREGION_MVCC_PRE_ASSIGN);
- if (this.mvccPreAssign) {
- this.preAssignMvccLock = new ReentrantLock();
- } else {
- this.preAssignMvccLock = null;
- }
}
void setHTableSpecificConf() {
@@ -3230,61 +3214,36 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
// STEP 4. Append the final edit to WAL and sync.
Mutation mutation = batchOp.getMutation(firstIndex);
WALKey walKey = null;
- long txid;
if (replay) {
// use wal key from the original
walKey = new ReplayHLogKey(this.getRegionInfo().getEncodedNameAsBytes(),
this.htableDescriptor.getTableName(), WALKey.NO_SEQUENCE_ID, now,
mutation.getClusterIds(), currentNonceGroup, currentNonce, mvcc);
walKey.setOrigLogSeqNum(batchOp.getReplaySequenceId());
- if (!walEdit.isEmpty()) {
- txid = this.wal.append(this.getRegionInfo(), walKey, walEdit, true);
- if (txid != 0) {
- sync(txid, durability);
- }
- }
- } else {
+ }
+ // Not sure what is going on here when replay is going on... does the below append get
+ // called for replayed edits? Am afraid to change it without test.
+ if (!walEdit.isEmpty()) {
+ if (!replay) {
+ // we use HLogKey here instead of WALKey directly to support legacy coprocessors.
+ walKey = new HLogKey(this.getRegionInfo().getEncodedNameAsBytes(),
+ this.htableDescriptor.getTableName(), WALKey.NO_SEQUENCE_ID, now,
+ mutation.getClusterIds(), currentNonceGroup, currentNonce, mvcc,
+ this.getReplicationScope());
+ }
+ // TODO: Use the doAppend methods below... complicated by the replay stuff above.
try {
- if (!walEdit.isEmpty()) {
- try {
- if (this.mvccPreAssign) {
- preAssignMvccLock.lock();
- writeEntry = mvcc.begin();
- }
- // we use HLogKey here instead of WALKey directly to support legacy coprocessors.
- walKey = new HLogKey(this.getRegionInfo().getEncodedNameAsBytes(),
- this.htableDescriptor.getTableName(), WALKey.NO_SEQUENCE_ID, now,
- mutation.getClusterIds(), currentNonceGroup, currentNonce, mvcc,
- this.getReplicationScope());
- if (this.mvccPreAssign) {
- walKey.setPreAssignedWriteEntry(writeEntry);
- }
- // TODO: Use the doAppend methods below... complicated by the replay stuff above.
- txid = this.wal.append(this.getRegionInfo(), walKey, walEdit, true);
- } finally {
- if (mvccPreAssign) {
- preAssignMvccLock.unlock();
- }
- }
- if (txid != 0) {
- sync(txid, durability);
- }
- if (writeEntry == null) {
- // if MVCC not preassigned, wait here until assigned
- writeEntry = walKey.getWriteEntry();
- }
- }
+ long txid = this.wal.append(this.getRegionInfo(), walKey,
+ walEdit, true);
+ if (txid != 0) sync(txid, durability);
+ writeEntry = walKey.getWriteEntry();
} catch (IOException ioe) {
- if (walKey != null && writeEntry == null) {
- // the writeEntry is not preassigned and error occurred during append or sync
- mvcc.complete(walKey.getWriteEntry());
- }
+ if (walKey != null) mvcc.complete(walKey.getWriteEntry());
throw ioe;
}
}
if (walKey == null) {
- // If no walKey, then not in replay and skipping WAL or some such. Begin an MVCC transaction
- // to get sequence id.
+ // If no walKey, then skipping WAL or some such. Being an mvcc transaction so sequenceid.
writeEntry = mvcc.begin();
}
@@ -3307,9 +3266,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
// STEP 6. Complete mvcc.
if (replay) {
this.mvcc.advanceTo(batchOp.getReplaySequenceId());
- } else {
- // writeEntry won't be empty if not in replay mode
- assert writeEntry != null;
+ } else if (writeEntry != null/*Can be null if in replay mode*/) {
mvcc.completeAndWait(writeEntry);
writeEntry = null;
}
@@ -7637,9 +7594,9 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
public static final long FIXED_OVERHEAD = ClassSize.align(
ClassSize.OBJECT +
ClassSize.ARRAY +
- 50 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
+ 49 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
(14 * Bytes.SIZEOF_LONG) +
- 6 * Bytes.SIZEOF_BOOLEAN);
+ 5 * Bytes.SIZEOF_BOOLEAN);
// woefully out of date - currently missing:
// 1 x HashMap - coprocessorServiceHandlers
http://git-wip-us.apache.org/repos/asf/hbase/blob/13baf4d3/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java
index c4546f5..72474a0 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java
@@ -112,16 +112,11 @@ class FSWALEntry extends Entry {
}
stamped = true;
long regionSequenceId = WALKey.NO_SEQUENCE_ID;
- WALKey key = getKey();
- MultiVersionConcurrencyControl.WriteEntry we = key.getPreAssignedWriteEntry();
- boolean preAssigned = (we != null);
- if (!preAssigned) {
- MultiVersionConcurrencyControl mvcc = key.getMvcc();
- if (mvcc != null) {
- we = mvcc.begin();
- }
- }
- if (we != null) {
+ MultiVersionConcurrencyControl mvcc = getKey().getMvcc();
+ MultiVersionConcurrencyControl.WriteEntry we = null;
+
+ if (mvcc != null) {
+ we = mvcc.begin();
regionSequenceId = we.getWriteNumber();
}
@@ -130,9 +125,7 @@ class FSWALEntry extends Entry {
CellUtil.setSequenceId(c, regionSequenceId);
}
}
- if (!preAssigned) {
- key.setWriteEntry(we);
- }
+ getKey().setWriteEntry(we);
return regionSequenceId;
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/13baf4d3/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java
index 4f2af38..0e35cbe 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALKey.java
@@ -42,7 +42,6 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos;
import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.FamilyScope;
import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.ScopeType;
import org.apache.hadoop.hbase.regionserver.MultiVersionConcurrencyControl;
-import org.apache.hadoop.hbase.regionserver.MultiVersionConcurrencyControl.WriteEntry;
import org.apache.hadoop.hbase.regionserver.SequenceId;
// imports for things that haven't moved from regionserver.wal yet.
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
@@ -93,10 +92,6 @@ public class WALKey implements SequenceId, Comparable<WALKey> {
*/
@InterfaceAudience.Private // For internal use only.
public MultiVersionConcurrencyControl.WriteEntry getWriteEntry() throws InterruptedIOException {
- if (this.preAssignedWriteEntry != null) {
- // don't wait for seqNumAssignedLatch if writeEntry is preassigned
- return this.preAssignedWriteEntry;
- }
try {
this.sequenceIdAssignedLatch.await();
} catch (InterruptedException ie) {
@@ -208,7 +203,6 @@ public class WALKey implements SequenceId, Comparable<WALKey> {
* Set in a way visible to multiple threads; e.g. synchronized getter/setters.
*/
private MultiVersionConcurrencyControl.WriteEntry writeEntry;
- private MultiVersionConcurrencyControl.WriteEntry preAssignedWriteEntry = null;
public static final List<UUID> EMPTY_UUIDS = Collections.unmodifiableList(new ArrayList<UUID>());
// visible for deprecated HLogKey
@@ -737,24 +731,4 @@ public class WALKey implements SequenceId, Comparable<WALKey> {
this.origLogSeqNum = walKey.getOrigSequenceNumber();
}
}
-
- /**
- * @return The preassigned writeEntry, if any
- */
- @InterfaceAudience.Private // For internal use only.
- public MultiVersionConcurrencyControl.WriteEntry getPreAssignedWriteEntry() {
- return this.preAssignedWriteEntry;
- }
-
- /**
- * Preassign writeEntry
- * @param writeEntry the entry to assign
- */
- @InterfaceAudience.Private // For internal use only.
- public void setPreAssignedWriteEntry(WriteEntry writeEntry) {
- if (writeEntry != null) {
- this.preAssignedWriteEntry = writeEntry;
- this.sequenceId = writeEntry.getWriteNumber();
- }
- }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/hbase/blob/13baf4d3/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
index 0b8eefa..612d6cf 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
@@ -6290,11 +6290,8 @@ public class TestHRegion {
@Override
public Long answer(InvocationOnMock invocation) throws Throwable {
WALKey key = invocation.getArgumentAt(1, WALKey.class);
- MultiVersionConcurrencyControl.WriteEntry we = key.getPreAssignedWriteEntry();
- if (we == null) {
- we = key.getMvcc().begin();
- key.setWriteEntry(we);
- }
+ MultiVersionConcurrencyControl.WriteEntry we = key.getMvcc().begin();
+ key.setWriteEntry(we);
return 1L;
}