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 na...@apache.org on 2019/04/10 12:30:22 UTC
[hadoop] branch trunk updated: HDDS-1401. Static ContainerCache in
Datanodes can result in overwrite of container db. Contributed by Mukul
Kumar Singh. (#708)
This is an automated email from the ASF dual-hosted git repository.
nanda pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new df01469 HDDS-1401. Static ContainerCache in Datanodes can result in overwrite of container db. Contributed by Mukul Kumar Singh. (#708)
df01469 is described below
commit df01469141e3933ca35785c25e1e29f59129cc85
Author: Mukul Kumar Singh <mk...@gmail.com>
AuthorDate: Wed Apr 10 18:00:10 2019 +0530
HDDS-1401. Static ContainerCache in Datanodes can result in overwrite of container db. Contributed by Mukul Kumar Singh. (#708)
---
.../container/common/utils/ContainerCache.java | 36 ++++++++++++----------
.../container/keyvalue/helpers/BlockUtils.java | 2 +-
.../container/keyvalue/impl/BlockManagerImpl.java | 2 +-
.../container/keyvalue/TestKeyValueContainer.java | 2 +-
.../hadoop/hdds/scm/block/BlockManagerImpl.java | 4 +--
.../common/impl/TestContainerPersistence.java | 2 +-
.../TestCloseContainerByPipeline.java | 17 ++++++++++
7 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
index a533684..25d1bdf 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerCache.java
@@ -69,15 +69,15 @@ public final class ContainerCache extends LRUMap {
/**
* Closes a db instance.
*
- * @param containerID - ID of the container to be closed.
+ * @param containerPath - path of the container db to be closed.
* @param db - db instance to close.
*/
- private void closeDB(long containerID, MetadataStore db) {
+ private void closeDB(String containerPath, MetadataStore db) {
if (db != null) {
try {
db.close();
- } catch (IOException e) {
- LOG.error("Error closing DB. Container: " + containerID, e);
+ } catch (Exception e) {
+ LOG.error("Error closing DB. Container: " + containerPath, e);
}
}
}
@@ -93,7 +93,7 @@ public final class ContainerCache extends LRUMap {
while (iterator.hasNext()) {
iterator.next();
MetadataStore db = (MetadataStore) iterator.getValue();
- closeDB(((Number)iterator.getKey()).longValue(), db);
+ closeDB((String)iterator.getKey(), db);
}
// reset the cache
cache.clear();
@@ -107,14 +107,18 @@ public final class ContainerCache extends LRUMap {
*/
@Override
protected boolean removeLRU(LinkEntry entry) {
+ MetadataStore db = (MetadataStore) entry.getValue();
+ String dbFile = (String)entry.getKey();
lock.lock();
try {
- MetadataStore db = (MetadataStore) entry.getValue();
- closeDB(((Number)entry.getKey()).longValue(), db);
+ closeDB(dbFile, db);
+ return true;
+ } catch (Exception e) {
+ LOG.error("Eviction for db:{} failed", dbFile, e);
+ return false;
} finally {
lock.unlock();
}
- return true;
}
/**
@@ -133,7 +137,7 @@ public final class ContainerCache extends LRUMap {
"Container ID cannot be negative.");
lock.lock();
try {
- MetadataStore db = (MetadataStore) this.get(containerID);
+ MetadataStore db = (MetadataStore) this.get(containerDBPath);
if (db == null) {
db = MetadataStoreBuilder.newBuilder()
@@ -142,7 +146,7 @@ public final class ContainerCache extends LRUMap {
.setConf(conf)
.setDBType(containerDBType)
.build();
- this.put(containerID, db);
+ this.put(containerDBPath, db);
}
return db;
} catch (Exception e) {
@@ -157,16 +161,14 @@ public final class ContainerCache extends LRUMap {
/**
* Remove a DB handler from cache.
*
- * @param containerID - ID of the container.
+ * @param containerPath - path of the container db file.
*/
- public void removeDB(long containerID) {
- Preconditions.checkState(containerID >= 0,
- "Container ID cannot be negative.");
+ public void removeDB(String containerPath) {
lock.lock();
try {
- MetadataStore db = (MetadataStore)this.get(containerID);
- closeDB(containerID, db);
- this.remove(containerID);
+ MetadataStore db = (MetadataStore)this.get(containerPath);
+ closeDB(containerPath, db);
+ this.remove(containerPath);
} finally {
lock.unlock();
}
diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java
index 200e8ea..996b592 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java
@@ -95,7 +95,7 @@ public final class BlockUtils {
Preconditions.checkNotNull(container);
ContainerCache cache = ContainerCache.getInstance(conf);
Preconditions.checkNotNull(cache);
- cache.removeDB(container.getContainerID());
+ cache.removeDB(container.getDbFile().getAbsolutePath());
}
/**
diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java
index 86865ac..3033dd9 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/BlockManagerImpl.java
@@ -160,7 +160,7 @@ public class BlockManagerImpl implements BlockManager {
}
byte[] kData = db.get(Longs.toByteArray(blockID.getLocalID()));
if (kData == null) {
- throw new StorageContainerException("Unable to find the block.",
+ throw new StorageContainerException("Unable to find the block." + blockID,
NO_SUCH_BLOCK);
}
ContainerProtos.BlockData blockData =
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
index 1aa7361..8e2986c 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
@@ -195,7 +195,7 @@ public class TestKeyValueContainer {
for (int i = 0; i < numberOfKeysToWrite; i++) {
metadataStore.put(("test" + i).getBytes(UTF_8), "test".getBytes(UTF_8));
}
- metadataStore.close();
+ BlockUtils.removeDB(keyValueContainerData, conf);
Map<String, String> metadata = new HashMap<>();
metadata.put("key1", "value1");
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
index d15f07b..0ca6670 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java
@@ -189,8 +189,8 @@ public class BlockManagerImpl implements BlockManager, BlockmanagerMXBean {
// factors are handled by pipeline creator
pipeline = pipelineManager.createPipeline(type, factor);
} catch (IOException e) {
- LOG.error("pipeline creation failed type:{} factor:{}", type,
- factor, e);
+ LOG.error("Pipeline creation failed for type:{} factor:{}",
+ type, factor, e);
break;
}
} else {
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
index f2e44cb..838dd9e 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
@@ -255,7 +255,7 @@ public class TestContainerPersistence {
"Container cannot be deleted because it is not empty.");
container2.delete();
Assert.assertTrue(containerSet.getContainerMapCopy()
- .containsKey(testContainerID1));
+ .containsKey(testContainerID2));
}
@Test
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestCloseContainerByPipeline.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestCloseContainerByPipeline.java
index fc6a74d..4a86f44 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestCloseContainerByPipeline.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestCloseContainerByPipeline.java
@@ -34,16 +34,21 @@ import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientFactory;
import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.hadoop.ozone.container.common.interfaces.Container;
+import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData;
+import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils;
import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo;
import org.apache.hadoop.ozone.protocol.commands.CloseContainerCommand;
import org.apache.hadoop.test.GenericTestUtils;
+import org.apache.hadoop.utils.MetadataStore;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.TimeoutException;
@@ -221,13 +226,25 @@ public class TestCloseContainerByPipeline {
List<DatanodeDetails> datanodes = pipeline.getNodes();
Assert.assertEquals(3, datanodes.size());
+ List<MetadataStore> metadataStores = new ArrayList<>(datanodes.size());
for (DatanodeDetails details : datanodes) {
Assert.assertFalse(isContainerClosed(cluster, containerID, details));
//send the order to close the container
cluster.getStorageContainerManager().getScmNodeManager()
.addDatanodeCommand(details.getUuid(),
new CloseContainerCommand(containerID, pipeline.getId()));
+ int index = cluster.getHddsDatanodeIndex(details);
+ Container dnContainer = cluster.getHddsDatanodes().get(index)
+ .getDatanodeStateMachine().getContainer().getContainerSet()
+ .getContainer(containerID);
+ metadataStores.add(BlockUtils.getDB((KeyValueContainerData) dnContainer
+ .getContainerData(), conf));
}
+
+ // There should be as many rocks db as the number of datanodes in pipeline.
+ Assert.assertEquals(datanodes.size(),
+ metadataStores.stream().distinct().count());
+
// Make sure that it is CLOSED
for (DatanodeDetails datanodeDetails : datanodes) {
GenericTestUtils.waitFor(
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org