Skip to content

HDDS-16460. Clear recovering entries when removing containers #11395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smengcl merged 4 commits into from
Oct 22, 2026
+64 −5
Merged

HDDS-16460. Clear recovering entries when removing containers #11395

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ccd783
HDDS-16460. Remove recovering entry when deleting container
rich7420 Sep 21, 2026
e783714
HDDS-16460. Centralize recovering container cleanup in ContainerSet
rich7420 Oct 5, 2026
08c5692
HDDS-16460. Address container cleanup review nits
rich7420 Oct 16, 2026
8e5ee03
Merge remote-tracking branch 'apache/master' into HDDS-16460
rich7420 Oct 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...ner-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ private boolean removeContainer(long containerId, boolean markMissing, boolean r
if (markMissing) {
missingContainerSet.add(containerId);
}
removeRecoveringContainer(containerId);
Container<?> removed = containerMap.remove(containerId);
if (removeFromDB) {
deleteFromContainerTable(containerId);
Expand Down
25 changes: 25 additions & 0 deletions ...service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,31 @@ public void testAddGetRemoveContainer(ContainerLayoutVersion layout)
assertFalse(containerSet.removeContainer(1000L));
}

@ContainerLayoutTestInfo.ContainerTest
public void testRemoveRecoveringContainerFromMemory(ContainerLayoutVersion layout) throws StorageContainerException {
ContainerSet containerSet = newContainerSet();
long containerId = 100L;
KeyValueContainerData data = new KeyValueContainerData(containerId, layout,
(long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), UUID.randomUUID().toString());
data.setState(ContainerProtos.ContainerDataProto.State.RECOVERING);
KeyValueContainer container = new KeyValueContainer(data, new OzoneConfiguration());
containerSet.addContainer(container);
assertThat(containerSet.getRecoveringContainerMap()).containsKey(containerId);

assertThat(containerSet.removeContainerOnlyFromMemory(containerId)).isTrue();
assertThat(containerSet.getContainer(containerId)).isNull();
assertThat(containerSet.getRecoveringContainerMap()).doesNotContainKey(containerId);

containerSet.addContainer(container);
assertThat(containerSet.getRecoveringContainerMap()).containsKey(containerId);
containerSet.removeContainerOnlyFromMemory(containerId);
data.closeContainer();
containerSet.addContainer(container);
assertThat(containerSet.getContainer(containerId).getContainerState())
.isEqualTo(ContainerProtos.ContainerDataProto.State.CLOSED);
assertThat(containerSet.getRecoveringContainerMap()).doesNotContainKey(containerId);
}

@ContainerLayoutTestInfo.ContainerTest
public void testIteratorsAndCount(ContainerLayoutVersion layout)
throws StorageContainerException {
Expand Down
16 changes: 12 additions & 4 deletions ...rc/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSetDiskChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.hadoop.ozone.container.common.volume;

import static org.apache.commons.lang3.RandomStringUtils.secure;
import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING;

@sreejasahithi sreejasahithi Oct 16, 2026

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.

nit: you are already importing "org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State" , so we can drop this and use State.RECOVERING everywhere

@rich7420 rich7420 Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

goood catch!

import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -44,6 +45,7 @@
import org.apache.hadoop.hdds.HddsConfigKeys;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State;
import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
import org.apache.hadoop.hdds.scm.net.HostAndPort;
Expand All @@ -65,6 +67,8 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;

/**
* Verify that {@link MutableVolumeSet} correctly checks for failed disks
Expand Down Expand Up @@ -248,8 +252,9 @@ private OzoneConfiguration getConfWithDataNodeDirs(int numDirs) {
* And FCR report is being sent.
* @throws IOException
*/
@Test
public void testVolumeFailure() throws IOException {
@ParameterizedTest
@EnumSource(value = State.class, names = {"CLOSED", "RECOVERING"})
public void testVolumeFailure(State state) throws IOException {
final int numVolumes = 5;

conf = getConfWithDataNodeDirs(numVolumes);
Expand All @@ -276,15 +281,15 @@ public void testVolumeFailure() throws IOException {
new KeyValueContainerData(containerID, layout,
ContainerTestHelper.CONTAINER_MAX_SIZE,
UUID.randomUUID().toString(), datanodeId.toString());
data.closeContainer();
data.setState(state);
data.setSchemaVersion(OzoneConsts.SCHEMA_V3);

long containerID1 = ContainerTestHelper.getTestContainerID();
KeyValueContainerData data1 =
new KeyValueContainerData(containerID1, layout,
ContainerTestHelper.CONTAINER_MAX_SIZE,
UUID.randomUUID().toString(), datanodeId.toString());
data1.closeContainer();
data1.setState(state);
data1.setSchemaVersion(OzoneConsts.SCHEMA_V3);

final MutableVolumeSet volumeSet = new MutableVolumeSet(
Expand All @@ -310,6 +315,7 @@ public void testVolumeFailure() throws IOException {
container1.create(volumeSet1,
new RoundRobinVolumeChoosingPolicy(), UUID.randomUUID().toString());
conSet.addContainer(container1);
assertThat(conSet.getRecoveringContainerMap()).hasSize(state == RECOVERING ? 2 : 0);
DatanodeStateMachine datanodeStateMachineMock =
mock(DatanodeStateMachine.class);
StateContext stateContext = new StateContext(
Expand All @@ -332,8 +338,10 @@ public void testVolumeFailure() throws IOException {
// ContainerID1 should be removed belonging to failed volume
assertNull(conSet.getContainer(containerID1));
assertTrue(conSet.getMissingContainerSet().contains(containerID1));
assertThat(conSet.getRecoveringContainerMap()).doesNotContainKey(containerID1);
// ContainerID should exist belonging to normal volume
assertNotNull(conSet.getContainer(containerID));
assertThat(conSet.getRecoveringContainerMap()).hasSize(state == RECOVERING ? 1 : 0);
expectedReportCount.put(
StorageContainerDatanodeProtocolProtos.ContainerReportsProto
.getDescriptor().getFullName(), 1);
Expand Down
26 changes: 26 additions & 0 deletions ...service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,32 @@ public void testDeleteContainer() throws IOException {
}
}

@Test
public void testDeleteRecoveringContainer() throws IOException {
HandlerWithVolumeSet handlerCtx = createKeyValueHandler(tempDir);
KeyValueHandler keyValueHandler = handlerCtx.getHandler();
ContainerSet containerSet = handlerCtx.getContainerSet();
try {
ContainerCommandRequestProto.Builder request = createContainerRequest(DATANODE_UUID, DUMMY_CONTAINER_ID)
.toBuilder();
request.getCreateContainerBuilder().setState(State.RECOVERING);
assertThat(keyValueHandler.handleCreateContainer(request.build(), null).getResult())
.isEqualTo(ContainerProtos.Result.SUCCESS);
Container<?> container = containerSet.getContainer(DUMMY_CONTAINER_ID);
assertThat(container).isNotNull();
assertThat(container.getContainerState()).isEqualTo(State.RECOVERING);
assertThat(containerSet.getRecoveringContainerMap()).containsKey(DUMMY_CONTAINER_ID);

keyValueHandler.deleteContainer(container, true);

assertThat(containerSet.getContainer(DUMMY_CONTAINER_ID)).isNull();
assertThat(containerSet.getRecoveringContainerMap()).doesNotContainKey(DUMMY_CONTAINER_ID);
} finally {
keyValueHandler.stop();
handlerCtx.getVolumeSet().getVolumesList().forEach(StorageVolume::shutdown);
}
}

/**
* Tests that deleting a container decrements the cached used space of its volume.
*/
Expand Down
Loading