You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ki...@apache.org on 2017/07/14 21:08:04 UTC
hadoop git commit: HDFS-12140. Remove BPOfferService lock contention
to get block pool id. Contributed by Daryn Sharp.
Repository: hadoop
Updated Branches:
refs/heads/trunk 8d86a9391 -> e7d187a1b
HDFS-12140. Remove BPOfferService lock contention to get block pool id. Contributed by Daryn Sharp.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/e7d187a1
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/e7d187a1
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/e7d187a1
Branch: refs/heads/trunk
Commit: e7d187a1b6a826edd5bd0f708184d48f3674d489
Parents: 8d86a93
Author: Kihwal Lee <ki...@apache.org>
Authored: Fri Jul 14 16:07:17 2017 -0500
Committer: Kihwal Lee <ki...@apache.org>
Committed: Fri Jul 14 16:07:17 2017 -0500
----------------------------------------------------------------------
.../hdfs/server/datanode/BPOfferService.java | 47 ++++++++++++++------
.../server/datanode/TestBPOfferService.java | 29 ++++++++++++
2 files changed, 63 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/e7d187a1/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
index 0384f26..dbf7c8d 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java
@@ -72,6 +72,7 @@ class BPOfferService {
volatile DatanodeRegistration bpRegistration;
private final String nameserviceId;
+ private volatile String bpId;
private final DataNode dn;
/**
@@ -184,6 +185,11 @@ class BPOfferService {
}
String getBlockPoolId(boolean quiet) {
+ // avoid lock contention unless the registration hasn't completed.
+ String id = bpId;
+ if (id != null) {
+ return id;
+ }
readLock();
try {
if (bpNSInfo != null) {
@@ -205,7 +211,7 @@ class BPOfferService {
}
boolean hasBlockPoolId() {
- return getNamespaceInfo() != null;
+ return getBlockPoolId(true) != null;
}
NamespaceInfo getNamespaceInfo() {
@@ -217,6 +223,28 @@ class BPOfferService {
}
}
+ @VisibleForTesting
+ NamespaceInfo setNamespaceInfo(NamespaceInfo nsInfo) throws IOException {
+ writeLock();
+ try {
+ NamespaceInfo old = bpNSInfo;
+ if (bpNSInfo != null && nsInfo != null) {
+ checkNSEquality(bpNSInfo.getBlockPoolID(), nsInfo.getBlockPoolID(),
+ "Blockpool ID");
+ checkNSEquality(bpNSInfo.getNamespaceID(), nsInfo.getNamespaceID(),
+ "Namespace ID");
+ checkNSEquality(bpNSInfo.getClusterID(), nsInfo.getClusterID(),
+ "Cluster ID");
+ }
+ bpNSInfo = nsInfo;
+ // cache the block pool id for lock-free access.
+ bpId = (nsInfo != null) ? nsInfo.getBlockPoolID() : null;
+ return old;
+ } finally {
+ writeUnlock();
+ }
+ }
+
@Override
public String toString() {
readLock();
@@ -289,9 +317,10 @@ class BPOfferService {
private void checkBlock(ExtendedBlock block) {
Preconditions.checkArgument(block != null,
"block is null");
- Preconditions.checkArgument(block.getBlockPoolId().equals(getBlockPoolId()),
+ final String bpId = getBlockPoolId();
+ Preconditions.checkArgument(block.getBlockPoolId().equals(bpId),
"block belongs to BP %s instead of BP %s",
- block.getBlockPoolId(), getBlockPoolId());
+ block.getBlockPoolId(), bpId);
}
//This must be called only by blockPoolManager
@@ -337,8 +366,7 @@ class BPOfferService {
}
try {
- if (this.bpNSInfo == null) {
- this.bpNSInfo = nsInfo;
+ if (setNamespaceInfo(nsInfo) == null) {
boolean success = false;
// Now that we know the namespace ID, etc, we can pass this to the DN.
@@ -352,16 +380,9 @@ class BPOfferService {
// The datanode failed to initialize the BP. We need to reset
// the namespace info so that other BPService actors still have
// a chance to set it, and re-initialize the datanode.
- this.bpNSInfo = null;
+ setNamespaceInfo(null);
}
}
- } else {
- checkNSEquality(bpNSInfo.getBlockPoolID(), nsInfo.getBlockPoolID(),
- "Blockpool ID");
- checkNSEquality(bpNSInfo.getNamespaceID(), nsInfo.getNamespaceID(),
- "Namespace ID");
- checkNSEquality(bpNSInfo.getClusterID(), nsInfo.getClusterID(),
- "Cluster ID");
}
} finally {
writeUnlock();
http://git-wip-us.apache.org/repos/asf/hadoop/blob/e7d187a1/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java
index aa47eeb..ec19926 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java
@@ -225,6 +225,35 @@ public class TestBPOfferService {
}
}
+ @Test
+ public void testLocklessBlockPoolId() throws Exception {
+ BPOfferService bpos = Mockito.spy(setupBPOSForNNs(mockNN1));
+
+ // bpNSInfo is not set, should take lock to check nsInfo.
+ assertNull(bpos.getBlockPoolId());
+ Mockito.verify(bpos).readLock();
+
+ // setting the bpNSInfo should cache the bp id, thus no locking.
+ Mockito.reset(bpos);
+ NamespaceInfo nsInfo = new NamespaceInfo(1, FAKE_CLUSTERID, FAKE_BPID, 0);
+ assertNull(bpos.setNamespaceInfo(nsInfo));
+ assertEquals(FAKE_BPID, bpos.getBlockPoolId());
+ Mockito.verify(bpos, Mockito.never()).readLock();
+
+ // clearing the bpNSInfo should clear the cached bp id, thus requiring
+ // locking to check the bpNSInfo.
+ Mockito.reset(bpos);
+ assertEquals(nsInfo, bpos.setNamespaceInfo(null));
+ assertNull(bpos.getBlockPoolId());
+ Mockito.verify(bpos).readLock();
+
+ // test setting it again.
+ Mockito.reset(bpos);
+ assertNull(bpos.setNamespaceInfo(nsInfo));
+ assertEquals(FAKE_BPID, bpos.getBlockPoolId());
+ Mockito.verify(bpos, Mockito.never()).readLock();
+ }
+
/**
* Test that DNA_INVALIDATE commands from the standby are ignored.
*/
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org