Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,12 @@ public final class ScmConfigKeys {
public static final String OZONE_SCM_PIPELINE_SCRUB_INTERVAL_DEFAULT =
"5m";

public static final String OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED =
"ozone.scm.remove.deleted.container.enabled";

public static final boolean
OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED_DEFAULT = true;


// Allow SCM to auto create factor ONE ratis pipeline.
public static final String OZONE_SCM_PIPELINE_AUTO_CREATE_FACTOR_ONE =
Expand Down
9 changes: 9 additions & 0 deletions hadoop-hdds/common/src/main/resources/ozone-default.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,15 @@
</description>
</property>

<property>
<name>ozone.scm.remove.deleted.container.enabled</name>
<value>true</value>
<tag>OZONE,SCM,OPERATION</tag>
<description>Boolean value to enable removing container reference from scm
if its state is DELETED
</description>
</property>

<property>
<name>hdds.metadata.dir</name>
<value/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private ContainerStateManagerImpl(final Configuration conf,
this.containerSize = getConfiguredContainerSize(conf);
this.containers = new ContainerStateMap();
this.lastUsedMap = new ConcurrentHashMap<>();
this.containerStateChangeActions = getContainerStateChangeActions();
this.containerStateChangeActions = getContainerStateChangeActions(conf);
this.transactionBuffer = buffer;
this.lockManager =
new LockManager<>(confSrc, true);
Expand Down Expand Up @@ -260,11 +260,23 @@ private void initialize() throws IOException {
}

private Map<LifeCycleEvent, CheckedConsumer<ContainerInfo, IOException>>
getContainerStateChangeActions() {
getContainerStateChangeActions(final Configuration conf) {
final Map<LifeCycleEvent, CheckedConsumer<ContainerInfo, IOException>>
actions = new EnumMap<>(LifeCycleEvent.class);
actions.put(FINALIZE, info -> pipelineManager
.removeContainerFromPipeline(info.getPipelineID(), info.containerID()));

boolean shouldRemoveDeletedContainer = conf.getBoolean(
ScmConfigKeys.OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED,
ScmConfigKeys.OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED_DEFAULT);
if (shouldRemoveDeletedContainer) {
// when a CLEANUP event if fired , the state of the container will be
// changed to DELETED. at this time, we should remove the reference
// of the container from scm. this happens immediately, thus DELETED
// is a transient state and hardly be captured.
actions.put(CLEANUP, info ->
removeContainer(info.containerID().getProtobuf()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is logged anywhere else, but could we ensure a lot message is written at INFO level saying container xxx has been deleted as it is empty.

}
return actions;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,8 @@ public enum MoveResult {
*/
private final MoveScheduler moveScheduler;

private final boolean shouldRemoveDeletedContainer;


/**
* Constructs ReplicationManager instance with the given configuration.
Expand Down Expand Up @@ -376,6 +378,10 @@ public LegacyReplicationManager(final ConfigurationSource conf,
.setDBTransactionBuffer(scmhaManager.getDBTransactionBuffer())
.setRatisServer(scmhaManager.getRatisServer())
.setMoveTable(moveTable).build();

shouldRemoveDeletedContainer = conf.getBoolean(
ScmConfigKeys.OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED,
ScmConfigKeys.OZONE_SCM_REMOVE_DELETED_CONTAINER_ENABLED_DEFAULT);
}


Expand Down Expand Up @@ -485,10 +491,16 @@ protected void processContainer(ContainerInfo container,
}

/**
* We don't need to take any action for a DELETE container - eventually
* it will be removed from SCM.
* a container reference should be removed from scm immediately
* after a CLEANUP event is fired, so DELETED is a transient
* state and hardly appears.if it appears at any unknown scenario,
* just remove it.
*/
if (state == LifeCycleState.DELETED) {
if (shouldRemoveDeletedContainer) {
LOG.warn("container {} is in DELETED state, but not removed", id);
containerManager.deleteContainer(id);
}
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ void testUpdateContainerState() throws Exception {
HddsProtos.LifeCycleEvent.FORCE_CLOSE);
Assertions.assertEquals(HddsProtos.LifeCycleState.CLOSED,
containerManager.getContainer(cid).getState());
containerManager.updateContainerState(cid,
HddsProtos.LifeCycleEvent.DELETE);
Assertions.assertEquals(HddsProtos.LifeCycleState.DELETING,
containerManager.getContainer(cid).getState());
Assertions.assertTrue(containerManager.containerExist(cid));
containerManager.updateContainerState(cid,
HddsProtos.LifeCycleEvent.CLEANUP);
//when a CLEANUP event is fired , the container should be
//removed from scm.
Assertions.assertFalse(containerManager.containerExist(cid));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,9 @@ public void testUpdateContainerState() throws IOException,
containerManager
.updateContainerState(container1.getContainerInfo().containerID(),
HddsProtos.LifeCycleEvent.CLEANUP);
containerList = containerStateManager
.getContainerIDs(HddsProtos.LifeCycleState.DELETED);
Assert.assertEquals(1, containerList.size());
//when CLEANUP is fired, container will be removed from scm
Assert.assertFalse(containerManager.containerExist(
container1.getContainerInfo().containerID()));

// Allocate container1 and update its state from
// OPEN -> CLOSING -> CLOSED
Expand All @@ -324,9 +324,6 @@ public void testUpdateContainerState() throws IOException,
containerManager
.updateContainerState(container3.getContainerInfo().containerID(),
HddsProtos.LifeCycleEvent.CLOSE);
containerManager
.updateContainerState(container1.getContainerInfo().containerID(),
HddsProtos.LifeCycleEvent.CLOSE);
containerList = containerStateManager
.getContainerIDs(HddsProtos.LifeCycleState.CLOSED);
Assert.assertEquals(1, containerList.size());
Expand Down