This is an automated email from the ASF dual-hosted git repository.
siddhant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new dd952d4aae HDDS-8008. Move pendingOps into ContainerStateManagerImpl
to ensure consistent state (#4298)
dd952d4aae is described below
commit dd952d4aaeb41a665c7b8a808abdaca5050219c6
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Mon Feb 27 06:36:04 2023 +0000
HDDS-8008. Move pendingOps into ContainerStateManagerImpl to ensure
consistent state (#4298)
---
.../hdds/scm/container/ContainerManagerImpl.java | 13 +--------
.../scm/container/ContainerStateManagerImpl.java | 33 ++++++++++++++++++----
.../scm/container/TestContainerReportHandler.java | 5 ++++
.../scm/container/TestContainerStateManager.java | 5 ++++
.../TestIncrementalContainerReportHandler.java | 5 ++++
.../replication/TestLegacyReplicationManager.java | 3 ++
6 files changed, 47 insertions(+), 17 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
index a3281efcf2..a4d6f36c59 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java
@@ -85,10 +85,6 @@ public class ContainerManagerImpl implements
ContainerManager {
@SuppressWarnings("java:S2245") // no need for secure random
private final Random random = new Random();
- // Used to track pending replication and delete for container replicas. In
- // ContainerManager, we try to remove any replicas we see added or deleted
- // in case they have been created by replication / delete command
- private final ContainerReplicaPendingOps containerReplicaPendingOps;
/**
*
@@ -112,6 +108,7 @@ public class ContainerManagerImpl implements
ContainerManager {
.setRatisServer(scmHaManager.getRatisServer())
.setContainerStore(containerStore)
.setSCMDBTransactionBuffer(scmHaManager.getDBTransactionBuffer())
+ .setContainerReplicaPendingOps(containerReplicaPendingOps)
.build();
this.numContainerPerVolume = conf
@@ -119,7 +116,6 @@ public class ContainerManagerImpl implements
ContainerManager {
ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT_DEFAULT);
this.scmContainerManagerMetrics = SCMContainerManagerMetrics.create();
- this.containerReplicaPendingOps = containerReplicaPendingOps;
}
@Override
@@ -332,9 +328,6 @@ public class ContainerManagerImpl implements
ContainerManager {
throws ContainerNotFoundException {
if (containerExist(cid)) {
containerStateManager.updateContainerReplica(cid, replica);
- // Clear any pending additions for this replica as we have now seen it.
- containerReplicaPendingOps.completeAddReplica(cid,
- replica.getDatanodeDetails(), replica.getReplicaIndex());
} else {
throwContainerNotFoundException(cid);
}
@@ -346,10 +339,6 @@ public class ContainerManagerImpl implements
ContainerManager {
throws ContainerNotFoundException, ContainerReplicaNotFoundException {
if (containerExist(cid)) {
containerStateManager.removeContainerReplica(cid, replica);
- // Remove any pending delete replication operations for the deleted
- // replica.
- containerReplicaPendingOps.completeDeleteReplica(cid,
- replica.getDatanodeDetails(), replica.getReplicaIndex());
} else {
throwContainerNotFoundException(cid);
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java
index 3f83d89c04..f79542885a 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManagerImpl.java
@@ -38,6 +38,7 @@ import
org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleEvent;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState;
import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import
org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps;
import org.apache.hadoop.hdds.scm.container.states.ContainerState;
import org.apache.hadoop.hdds.scm.container.states.ContainerStateMap;
import org.apache.hadoop.hdds.scm.ha.CheckedConsumer;
@@ -122,6 +123,13 @@ public final class ContainerStateManagerImpl
*/
private final StateMachine<LifeCycleState, LifeCycleEvent> stateMachine;
+ /**
+ * Pending Ops table used by Replication Manager to track pending moves. As
+ * replicas are added or removed, we make a call to the pendingOps object to
+ * mark the pending operations as completed.
+ */
+ private final ContainerReplicaPendingOps containerReplicaPendingOps;
+
/**
* We use the containers in round-robin fashion for operations like block
* allocation. This map is used for remembering the last used container.
@@ -147,8 +155,8 @@ public final class ContainerStateManagerImpl
private ContainerStateManagerImpl(final Configuration conf,
final PipelineManager pipelineManager,
final Table<ContainerID, ContainerInfo> containerStore,
- final DBTransactionBuffer buffer)
- throws IOException {
+ final DBTransactionBuffer buffer,
+ final ContainerReplicaPendingOps pendingOps) throws IOException {
this.confSrc = OzoneConfiguration.of(conf);
this.pipelineManager = pipelineManager;
this.containerStore = containerStore;
@@ -160,6 +168,7 @@ public final class ContainerStateManagerImpl
this.transactionBuffer = buffer;
this.lockManager =
new LockManager<>(confSrc, true);
+ this.containerReplicaPendingOps = pendingOps;
initialize();
}
@@ -402,6 +411,9 @@ public final class ContainerStateManagerImpl
lockManager.writeLock(id);
try {
containers.updateContainerReplica(id, replica);
+ // Clear any pending additions for this replica as we have now seen it.
+ containerReplicaPendingOps.completeAddReplica(id,
+ replica.getDatanodeDetails(), replica.getReplicaIndex());
} finally {
lockManager.writeUnlock(id);
}
@@ -412,8 +424,11 @@ public final class ContainerStateManagerImpl
final ContainerReplica replica) {
lockManager.writeLock(id);
try {
- containers.removeContainerReplica(id,
- replica);
+ containers.removeContainerReplica(id, replica);
+ // Remove any pending delete replication operations for the deleted
+ // replica.
+ containerReplicaPendingOps.completeDeleteReplica(id,
+ replica.getDatanodeDetails(), replica.getReplicaIndex());
} finally {
lockManager.writeUnlock(id);
}
@@ -562,6 +577,7 @@ public final class ContainerStateManagerImpl
private SCMRatisServer scmRatisServer;
private Table<ContainerID, ContainerInfo> table;
private DBTransactionBuffer transactionBuffer;
+ private ContainerReplicaPendingOps containerReplicaPendingOps;
public Builder setSCMDBTransactionBuffer(DBTransactionBuffer buffer) {
this.transactionBuffer = buffer;
@@ -588,13 +604,20 @@ public final class ContainerStateManagerImpl
return this;
}
+ public Builder setContainerReplicaPendingOps(
+ final ContainerReplicaPendingOps pendingOps) {
+ containerReplicaPendingOps = pendingOps;
+ return this;
+ }
+
public ContainerStateManager build() throws IOException {
Preconditions.checkNotNull(conf);
Preconditions.checkNotNull(pipelineMgr);
Preconditions.checkNotNull(table);
final ContainerStateManager csm = new ContainerStateManagerImpl(
- conf, pipelineMgr, table, transactionBuffer);
+ conf, pipelineMgr, table, transactionBuffer,
+ containerReplicaPendingOps);
final SCMHAInvocationHandler invocationHandler =
new SCMHAInvocationHandler(RequestType.CONTAINER, csm,
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
index a48f3470a3..469b1ddf24 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
@@ -29,6 +29,7 @@ import org.apache.hadoop.hdds.protocol.proto
import org.apache.hadoop.hdds.protocol.proto
.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto;
import org.apache.hadoop.hdds.scm.HddsTestUtils;
+import
org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps;
import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub;
import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
import org.apache.hadoop.hdds.scm.metadata.SCMDBDefinition;
@@ -57,6 +58,8 @@ import org.mockito.Mockito;
import java.io.File;
import java.io.IOException;
+import java.time.Clock;
+import java.time.ZoneId;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
@@ -106,6 +109,8 @@ public class TestContainerReportHandler {
.setRatisServer(scmhaManager.getRatisServer())
.setContainerStore(SCMDBDefinition.CONTAINERS.getTable(dbStore))
.setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setContainerReplicaPendingOps(new ContainerReplicaPendingOps(
+ conf, Clock.system(ZoneId.systemDefault())))
.build();
publisher = Mockito.mock(EventPublisher.class);
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManager.java
index e731e90b33..1b5f6a3f00 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerStateManager.java
@@ -19,6 +19,8 @@ package org.apache.hadoop.hdds.scm.container;
import java.io.File;
import java.io.IOException;
+import java.time.Clock;
+import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Set;
import java.util.UUID;
@@ -35,6 +37,7 @@ import
org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor;
import org.apache.hadoop.hdds.protocol.proto
.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto;
+import
org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps;
import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub;
import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
import org.apache.hadoop.hdds.scm.metadata.SCMDBDefinition;
@@ -92,6 +95,8 @@ public class TestContainerStateManager {
.setRatisServer(scmhaManager.getRatisServer())
.setContainerStore(SCMDBDefinition.CONTAINERS.getTable(dbStore))
.setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setContainerReplicaPendingOps(new ContainerReplicaPendingOps(
+ conf, Clock.system(ZoneId.systemDefault())))
.build();
}
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestIncrementalContainerReportHandler.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestIncrementalContainerReportHandler.java
index 47b932a12e..c7928b21d2 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestIncrementalContainerReportHandler.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestIncrementalContainerReportHandler.java
@@ -30,6 +30,7 @@ import org.apache.hadoop.hdds.protocol.proto
import org.apache.hadoop.hdds.protocol.proto
.StorageContainerDatanodeProtocolProtos.IncrementalContainerReportProto;
import org.apache.hadoop.hdds.scm.HddsTestUtils;
+import
org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps;
import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub;
import org.apache.hadoop.hdds.scm.ha.SCMContext;
import org.apache.hadoop.hdds.scm.ha.SCMHAManager;
@@ -64,6 +65,8 @@ import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.time.Clock;
+import java.time.ZoneId;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -136,6 +139,8 @@ public class TestIncrementalContainerReportHandler {
.setRatisServer(scmhaManager.getRatisServer())
.setContainerStore(SCMDBDefinition.CONTAINERS.getTable(dbStore))
.setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setContainerReplicaPendingOps(new ContainerReplicaPendingOps(
+ conf, Clock.system(ZoneId.systemDefault())))
.build();
this.publisher = Mockito.mock(EventPublisher.class);
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java
index eb1e8c97ea..6afb362611 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestLegacyReplicationManager.java
@@ -78,6 +78,7 @@ import org.mockito.Mockito;
import java.io.File;
import java.io.IOException;
+import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.ArrayList;
@@ -172,6 +173,8 @@ public class TestLegacyReplicationManager {
.setRatisServer(scmhaManager.getRatisServer())
.setContainerStore(SCMDBDefinition.CONTAINERS.getTable(dbStore))
.setSCMDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
+ .setContainerReplicaPendingOps(new ContainerReplicaPendingOps(
+ conf, Clock.system(ZoneId.systemDefault())))
.build();
serviceManager = new SCMServiceManager();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
