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 c3beeb7 HDDS-2048: State check during container state transition in
datanode should be lock protected (#1375)
c3beeb7 is described below
commit c3beeb7761a08f57ad1d45a2d31b4f8a35ff67d9
Author: Lokesh Jain <lj...@apache.org>
AuthorDate: Tue Sep 10 14:14:52 2019 +0530
HDDS-2048: State check during container state transition in datanode should
be lock protected (#1375)
---
.../container/keyvalue/KeyValueContainer.java | 6 +-
.../ozone/container/keyvalue/KeyValueHandler.java | 120 ++++++++++++---------
.../container/keyvalue/impl/BlockManagerImpl.java | 32 +++---
3 files changed, 94 insertions(+), 64 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
index ff57037..a6e914b 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
@@ -82,7 +82,9 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Class to perform KeyValue Container operations.
+ * Class to perform KeyValue Container operations. Any modifications to
+ * KeyValueContainer object should ideally be done via api exposed in
+ * KeyValueHandler class.
*/
public class KeyValueContainer implements Container<KeyValueContainerData> {
@@ -554,6 +556,8 @@ public class KeyValueContainer implements
Container<KeyValueContainerData> {
* Acquire write lock.
*/
public void writeLock() {
+ // TODO: The lock for KeyValueContainer object should not be exposed
+ // publicly.
this.lock.writeLock().lock();
}
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
index ab1d124..f39973f 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
@@ -881,75 +881,97 @@ public class KeyValueHandler extends Handler {
@Override
public void markContainerForClose(Container container)
throws IOException {
- // Move the container to CLOSING state only if it's OPEN
- if (container.getContainerState() == State.OPEN) {
- container.markContainerForClose();
- sendICR(container);
+ container.writeLock();
+ try {
+ // Move the container to CLOSING state only if it's OPEN
+ if (container.getContainerState() == State.OPEN) {
+ container.markContainerForClose();
+ sendICR(container);
+ }
+ } finally {
+ container.writeUnlock();
}
}
@Override
public void markContainerUnhealthy(Container container)
throws IOException {
- if (container.getContainerState() != State.UNHEALTHY) {
- try {
- container.markContainerUnhealthy();
- } catch (IOException ex) {
- // explicitly catch IOException here since the this operation
- // will fail if the Rocksdb metadata is corrupted.
- long id = container.getContainerData().getContainerID();
- LOG.warn("Unexpected error while marking container "
- +id+ " as unhealthy", ex);
- } finally {
- sendICR(container);
+ container.writeLock();
+ try {
+ if (container.getContainerState() != State.UNHEALTHY) {
+ try {
+ container.markContainerUnhealthy();
+ } catch (IOException ex) {
+ // explicitly catch IOException here since the this operation
+ // will fail if the Rocksdb metadata is corrupted.
+ long id = container.getContainerData().getContainerID();
+ LOG.warn("Unexpected error while marking container " + id
+ + " as unhealthy", ex);
+ } finally {
+ sendICR(container);
+ }
}
+ } finally {
+ container.writeUnlock();
}
}
@Override
public void quasiCloseContainer(Container container)
throws IOException {
- final State state = container.getContainerState();
- // Quasi close call is idempotent.
- if (state == State.QUASI_CLOSED) {
- return;
- }
- // The container has to be in CLOSING state.
- if (state != State.CLOSING) {
- ContainerProtos.Result error = state == State.INVALID ?
- INVALID_CONTAINER_STATE : CONTAINER_INTERNAL_ERROR;
- throw new StorageContainerException("Cannot quasi close container #" +
- container.getContainerData().getContainerID() + " while in " +
- state + " state.", error);
+ container.writeLock();
+ try {
+ final State state = container.getContainerState();
+ // Quasi close call is idempotent.
+ if (state == State.QUASI_CLOSED) {
+ return;
+ }
+ // The container has to be in CLOSING state.
+ if (state != State.CLOSING) {
+ ContainerProtos.Result error =
+ state == State.INVALID ? INVALID_CONTAINER_STATE :
+ CONTAINER_INTERNAL_ERROR;
+ throw new StorageContainerException(
+ "Cannot quasi close container #" + container.getContainerData()
+ .getContainerID() + " while in " + state + " state.", error);
+ }
+ container.quasiClose();
+ sendICR(container);
+ } finally {
+ container.writeUnlock();
}
- container.quasiClose();
- sendICR(container);
}
@Override
public void closeContainer(Container container)
throws IOException {
- final State state = container.getContainerState();
- // Close call is idempotent.
- if (state == State.CLOSED) {
- return;
+ container.writeLock();
+ try {
+ final State state = container.getContainerState();
+ // Close call is idempotent.
+ if (state == State.CLOSED) {
+ return;
+ }
+ if (state == State.UNHEALTHY) {
+ throw new StorageContainerException(
+ "Cannot close container #" + container.getContainerData()
+ .getContainerID() + " while in " + state + " state.",
+ ContainerProtos.Result.CONTAINER_UNHEALTHY);
+ }
+ // The container has to be either in CLOSING or in QUASI_CLOSED state.
+ if (state != State.CLOSING && state != State.QUASI_CLOSED) {
+ ContainerProtos.Result error =
+ state == State.INVALID ? INVALID_CONTAINER_STATE :
+ CONTAINER_INTERNAL_ERROR;
+ throw new StorageContainerException(
+ "Cannot close container #" + container.getContainerData()
+ .getContainerID() + " while in " + state + " state.", error);
+ }
+ container.close();
+ sendICR(container);
+ } finally {
+ container.writeUnlock();
}
- if (state == State.UNHEALTHY) {
- throw new StorageContainerException(
- "Cannot close container #" + container.getContainerData()
- .getContainerID() + " while in " + state + " state.",
- ContainerProtos.Result.CONTAINER_UNHEALTHY);
- }
- // The container has to be either in CLOSING or in QUASI_CLOSED state.
- if (state != State.CLOSING && state != State.QUASI_CLOSED) {
- ContainerProtos.Result error = state == State.INVALID ?
- INVALID_CONTAINER_STATE : CONTAINER_INTERNAL_ERROR;
- throw new StorageContainerException("Cannot close container #" +
- container.getContainerData().getContainerID() + " while in " +
- state + " state.", error);
- }
- container.close();
- sendICR(container);
}
@Override
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 53715c8..fb28cf8 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
@@ -258,21 +258,25 @@ public class BlockManagerImpl implements BlockManager {
Preconditions.checkArgument(count > 0,
"Count must be a positive number.");
container.readLock();
- List<BlockData> result = null;
- KeyValueContainerData cData = (KeyValueContainerData) container
- .getContainerData();
- try(ReferenceCountedDB db = BlockUtils.getDB(cData, config)) {
- result = new ArrayList<>();
- byte[] startKeyInBytes = Longs.toByteArray(startLocalID);
- List<Map.Entry<byte[], byte[]>> range =
- db.getStore().getSequentialRangeKVs(startKeyInBytes, count,
- MetadataKeyFilters.getNormalKeyFilter());
- for (Map.Entry<byte[], byte[]> entry : range) {
- BlockData value = BlockUtils.getBlockData(entry.getValue());
- BlockData data = new BlockData(value.getBlockID());
- result.add(data);
+ try {
+ List<BlockData> result = null;
+ KeyValueContainerData cData =
+ (KeyValueContainerData) container.getContainerData();
+ try (ReferenceCountedDB db = BlockUtils.getDB(cData, config)) {
+ result = new ArrayList<>();
+ byte[] startKeyInBytes = Longs.toByteArray(startLocalID);
+ List<Map.Entry<byte[], byte[]>> range = db.getStore()
+ .getSequentialRangeKVs(startKeyInBytes, count,
+ MetadataKeyFilters.getNormalKeyFilter());
+ for (Map.Entry<byte[], byte[]> entry : range) {
+ BlockData value = BlockUtils.getBlockData(entry.getValue());
+ BlockData data = new BlockData(value.getBlockID());
+ result.add(data);
+ }
+ return result;
}
- return result;
+ } finally {
+ container.readUnlock();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org
