Skip to content

HDDS-15791. EC reconstructed RECOVERING container while still idle can be deleted by SCM and recreated as new partial OPEN container #10702

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

Open
aswinshakil wants to merge 2 commits into
base: master
Choose a base branch
from
Open

HDDS-15791. EC reconstructed RECOVERING container while still idle can be deleted by SCM and recreated as new partial OPEN container #10702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
57b63ce
HDDS-15791. EC reconstructed RECOVERING container while still idle ca…
aswinshakil Jul 9, 2026
e8355a2
HDDS-15791. Address Review Comment
aswinshakil Jul 10, 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
15 changes: 11 additions & 4 deletions hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,11 +907,11 @@ private CompletableFuture<PutBlockResult> writeChunkToContainer(
ChecksumData checksumData = checksum.computeChecksum(chunk, false);
// side note: checksum object is shared with PutBlock's (blockData) checksum calc,
// current impl does not support caching both
ChunkInfo chunkInfo = ChunkInfo.newBuilder()
ChunkInfo chunkInfo = decorateChunkInfo(ChunkInfo.newBuilder()
.setChunkName(blockID.get().getLocalID() + "_chunk_" + ++chunkIndex)

@devmadhuu devmadhuu Jul 10, 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.

decorateChunkInfo method will fall into both normal EC write path and reconstruction path. Kindly check this part.

@aswinshakil aswinshakil Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated this.

.setOffset(offset)
.setLen(effectiveChunkSize)
.setChecksumData(checksumData.getProtoBufMessage())
.setChecksumData(checksumData.getProtoBufMessage()))
.build();

long flushPos = totalWriteChunkLength;
Expand Down Expand Up @@ -1148,18 +1148,25 @@ private ChunkInfo createChunkInfo(long lastPartialChunkOffset)
ChecksumData revisedChecksumData = checksum.computeChecksum(lastChunkBuffer, true);

long chunkID = lastPartialChunkOffset / config.getStreamBufferSize();
ChunkInfo.Builder revisedChunkInfo = ChunkInfo.newBuilder()
ChunkInfo.Builder revisedChunkInfo = decorateChunkInfo(ChunkInfo.newBuilder()
.setChunkName(blockID.get().getLocalID() + "_chunk_" + chunkID)
.setOffset(lastPartialChunkOffset)
.setLen(revisedChunkSize)
.setChecksumData(revisedChecksumData.getProtoBufMessage());
.setChecksumData(revisedChecksumData.getProtoBufMessage()));
// if full chunk
if (revisedChunkSize == config.getStreamBufferSize()) {
revisedChunkInfo.addMetadata(FULL_CHUNK_KV);
}
return revisedChunkInfo.build();
}

/**
* Subclasses may add chunk metadata (e.g. EC reconstruction write flags).
*/
protected ChunkInfo.Builder decorateChunkInfo(ChunkInfo.Builder builder) {
return builder;
}

private boolean isFullChunk(ChunkInfo chunkInfo) {
Preconditions.checkState(
chunkInfo.getLen() <= config.getStreamBufferSize());
Expand Down
37 changes: 37 additions & 0 deletions hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/ECBlockOutputStream.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.hdds.scm.storage;

import static org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls.putBlockAsync;
import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_CREATABLE_KEY;

import com.google.common.base.Preconditions;
import java.io.IOException;
Expand Down Expand Up @@ -59,6 +60,7 @@
public class ECBlockOutputStream extends BlockOutputStream {

private final DatanodeDetails datanodeDetails;
private final boolean denyContainerAutoCreate;
private CompletableFuture<ContainerProtos.ContainerCommandResponseProto>
currentChunkRspFuture = null;

Expand All @@ -83,11 +85,46 @@ public ECBlockOutputStream(
Token<? extends TokenIdentifier> token,
ContainerClientMetrics clientMetrics, StreamBufferArgs streamBufferArgs,
Supplier<ExecutorService> executorServiceSupplier
) throws IOException {
this(blockID, xceiverClientManager, pipeline, bufferPool, config, token, clientMetrics,
streamBufferArgs, executorServiceSupplier, false);
}

@SuppressWarnings("checkstyle:ParameterNumber")
public ECBlockOutputStream(
BlockID blockID,
XceiverClientFactory xceiverClientManager,
Pipeline pipeline,
BufferPool bufferPool,
OzoneClientConfig config,
Token<? extends TokenIdentifier> token,
ContainerClientMetrics clientMetrics, StreamBufferArgs streamBufferArgs,
Supplier<ExecutorService> executorServiceSupplier,
boolean denyContainerAutoCreate
) throws IOException {
super(blockID, -1, xceiverClientManager,
pipeline, bufferPool, config, token, clientMetrics, streamBufferArgs, executorServiceSupplier);
// In EC stream, there will be only one node in pipeline.
this.datanodeDetails = pipeline.getClosestNode();
this.denyContainerAutoCreate = denyContainerAutoCreate;
if (denyContainerAutoCreate) {
getContainerBlockData().addMetadata(containerCreatableFalseKv());
}
}

@Override
protected ChunkInfo.Builder decorateChunkInfo(ChunkInfo.Builder builder) {

@devmadhuu devmadhuu Jul 10, 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.

I think, this might break the normal EC write as well because ECBlockOutputStream is used by both the reconstruction coordinator (ECReconstructionCoordinator) and the normal client write path (ECBlockOutputStreamEntry). So we might need to have this flag CONTAINER_CREATABLE_FALSE only for reconstruction path.

@aswinshakil aswinshakil Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated to add overloaded API for reconstruction.

if (!denyContainerAutoCreate) {
return builder;
}
return builder.addMetadata(containerCreatableFalseKv());
}

private static ContainerProtos.KeyValue containerCreatableFalseKv() {
return ContainerProtos.KeyValue.newBuilder()
.setKey(CONTAINER_CREATABLE_KEY)
.setValue(OzoneConsts.CONTAINER_CREATABLE)
.build();
}

@Override
Expand Down
78 changes: 76 additions & 2 deletions ...nt/src/test/java/org/apache/hadoop/hdds/scm/storage/TestBlockOutputStreamCorrectness.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
package org.apache.hadoop.hdds.scm.storage;

import static java.util.concurrent.Executors.newFixedThreadPool;
import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_CREATABLE;
import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_CREATABLE_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -34,6 +38,7 @@
import org.apache.hadoop.hdds.client.ECReplicationConfig;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ChecksumType;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto;
Expand All @@ -52,6 +57,7 @@
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.ozone.ClientVersion;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.common.Checksum;
import org.apache.hadoop.ozone.container.common.helpers.BlockData;
import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
Expand Down Expand Up @@ -132,6 +138,68 @@ public void testMissingStripeChecksumDoesNotMakeExecutePutBlockFailDuringECRecon
}
}

@Test
public void testEcReconstructionWriteChunkSetsContainerCreatableFalse() throws IOException {
OzoneClientConfig config = new OzoneClientConfig();
ECReplicationConfig replicationConfig = new ECReplicationConfig(3, 2);
BlockID blockID = new BlockID(1, 1);
DatanodeDetails datanodeDetails = MockDatanodeDetails.randomDatanodeDetails();
Pipeline pipeline = Pipeline.newBuilder()
.setId(datanodeDetails.getID())
.setReplicationConfig(replicationConfig)
.setNodes(ImmutableList.of(datanodeDetails))
.setState(Pipeline.PipelineState.CLOSED)
.setReplicaIndexes(ImmutableMap.of(datanodeDetails, 2))
.build();

try (ECBlockOutputStream ecBlockOutputStream = createECBlockOutputStream(config, replicationConfig,
blockID, pipeline, true)) {
ContainerProtos.ChunkInfo chunk = ecBlockOutputStream.decorateChunkInfo(
ContainerProtos.ChunkInfo.newBuilder()
.setChunkName("chunk")
.setOffset(0)
.setLen(1)
.setChecksumData(Checksum.getNoChecksumDataProto()))
.build();
assertTrue(chunk.getMetadataList().stream()
.anyMatch(kv -> CONTAINER_CREATABLE_KEY.equals(kv.getKey())
&& CONTAINER_CREATABLE.equals(kv.getValue())));
assertTrue(ecBlockOutputStream.getContainerBlockData().getMetadataList().stream()
.anyMatch(kv -> CONTAINER_CREATABLE_KEY.equals(kv.getKey())
&& CONTAINER_CREATABLE.equals(kv.getValue())));
}
}

@Test
public void testEcClientWriteChunkDoesNotSetContainerCreatableFalse() throws IOException {
OzoneClientConfig config = new OzoneClientConfig();
ECReplicationConfig replicationConfig = new ECReplicationConfig(3, 2);
BlockID blockID = new BlockID(1, 1);
DatanodeDetails datanodeDetails = MockDatanodeDetails.randomDatanodeDetails();
Pipeline pipeline = Pipeline.newBuilder()
.setId(datanodeDetails.getID())
.setReplicationConfig(replicationConfig)
.setNodes(ImmutableList.of(datanodeDetails))
.setState(Pipeline.PipelineState.CLOSED)
.setReplicaIndexes(ImmutableMap.of(datanodeDetails, 2))
.build();

try (ECBlockOutputStream ecBlockOutputStream = createECBlockOutputStream(config, replicationConfig,
blockID, pipeline, false)) {
ContainerProtos.ChunkInfo chunk = ecBlockOutputStream.decorateChunkInfo(
ContainerProtos.ChunkInfo.newBuilder()
.setChunkName("chunk")
.setOffset(0)
.setLen(1)
.setChecksumData(Checksum.getNoChecksumDataProto()))
.build();
assertFalse(chunk.getMetadataList().stream()
.anyMatch(kv -> CONTAINER_CREATABLE_KEY.equals(kv.getKey())));
assertFalse(ecBlockOutputStream.getContainerBlockData().getMetadataList().stream()
.anyMatch(kv -> CONTAINER_CREATABLE_KEY.equals(kv.getKey())));
}
}

/**
* Creates a BlockData array with {@link ECReplicationConfig#getRequiredNodes()} number of elements.
*/
Expand Down Expand Up @@ -183,7 +251,8 @@ private BlockOutputStream createBlockOutputStream(BufferPool bufferPool)
}

private ECBlockOutputStream createECBlockOutputStream(OzoneClientConfig clientConfig,
ECReplicationConfig repConfig, BlockID blockID, Pipeline pipeline) throws IOException {
ECReplicationConfig repConfig, BlockID blockID, Pipeline pipeline,
boolean denyContainerAutoCreate) throws IOException {
final XceiverClientManager xcm = mock(XceiverClientManager.class);
when(xcm.acquireClient(any()))
.thenReturn(new MockXceiverClientSpi(pipeline));
Expand All @@ -193,7 +262,12 @@ private ECBlockOutputStream createECBlockOutputStream(OzoneClientConfig clientCo
StreamBufferArgs.getDefaultStreamBufferArgs(repConfig, clientConfig);

return new ECBlockOutputStream(blockID, xcm, pipeline, BufferPool.empty(), clientConfig, null,
clientMetrics, streamBufferArgs, () -> newFixedThreadPool(2));
clientMetrics, streamBufferArgs, () -> newFixedThreadPool(2), denyContainerAutoCreate);
}

private ECBlockOutputStream createECBlockOutputStream(OzoneClientConfig clientConfig,
ECReplicationConfig repConfig, BlockID blockID, Pipeline pipeline) throws IOException {
return createECBlockOutputStream(clientConfig, repConfig, blockID, pipeline, false);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ public final class OzoneConsts {
public static final int CHUNK_SIZE = 1 * 1024 * 1024; // 1 MB
// for client and DataNode to label a block contains a incremental chunk list.
public static final String INCREMENTAL_CHUNK_LIST = "incremental";
/**
* Write metadata value for key {@code containerCreatable}: the DataNode must not
* auto-create a missing container (EC reconstruction writes).
*/
public static final String CONTAINER_CREATABLE_KEY = "containerCreatable";
public static final String CONTAINER_CREATABLE = "false";
public static final long KB = 1024L;
public static final long MB = KB * 1024L;
public static final long GB = MB * 1024L;
Expand Down
52 changes: 52 additions & 0 deletions ...ervice/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.NO_SUCH_ALGORITHM;
import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.UNABLE_TO_FIND_DATA_DIR;
import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getContainerCommandResponse;
import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_CREATABLE_KEY;
import static org.apache.hadoop.ozone.container.common.impl.ContainerData.CHARSET_ENCODING;

import java.io.File;
Expand All @@ -35,6 +36,7 @@
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.Properties;
import java.util.UUID;
Expand Down Expand Up @@ -403,4 +405,54 @@ public static long getPendingDeletionBytes(ContainerData containerData) {
" not support.");
}
}

public static ContainerProtos.KeyValue containerCreatableFalseKv() {
return ContainerProtos.KeyValue.newBuilder()
.setKey(CONTAINER_CREATABLE_KEY)
.setValue(OzoneConsts.CONTAINER_CREATABLE)
.build();
}

/**
* @return true if the DataNode may auto-create a missing container for this request
*/
public static boolean isContainerCreatable(ContainerCommandRequestProto request) {

@devmadhuu devmadhuu Jul 10, 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.

This flag is currently only restricted to check writeChunk metadata only, but missing PutBlock / PutSmallFilecheck. In a partial/padding stripe, non-writing nodes do create containers by an empty PutBlock, so in normal EC writepath, it is fine. But in case of reconstruction path, a slight edge case where PutBlock
might land up on a force-deleted RECOVERING container can still auto-create it as OPEN

@aswinshakil aswinshakil Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added for PutBlock as well.

if (request.hasWriteChunk()) {
ContainerProtos.WriteChunkRequestProto writeChunk = request.getWriteChunk();
if (writeChunk.hasChunkData()
&& deniesContainerAutoCreate(writeChunk.getChunkData().getMetadataList())) {
return false;
}
}
if (request.hasPutBlock()) {
ContainerProtos.PutBlockRequestProto putBlock = request.getPutBlock();
if (putBlock.hasBlockData()
&& deniesContainerAutoCreate(putBlock.getBlockData().getMetadataList())) {
return false;
}
}
if (request.hasPutSmallFile()) {
ContainerProtos.PutSmallFileRequestProto putSmallFile = request.getPutSmallFile();
if (putSmallFile.hasChunkInfo()
&& deniesContainerAutoCreate(putSmallFile.getChunkInfo().getMetadataList())) {
return false;
}
if (putSmallFile.hasBlock() && putSmallFile.getBlock().hasBlockData()
&& deniesContainerAutoCreate(putSmallFile.getBlock().getBlockData().getMetadataList())) {
return false;
}
}
return true;
}

private static boolean deniesContainerAutoCreate(
List<ContainerProtos.KeyValue> metadataList) {
for (ContainerProtos.KeyValue kv : metadataList) {
if (CONTAINER_CREATABLE_KEY.equals(kv.getKey())
&& OzoneConsts.CONTAINER_CREATABLE.equals(kv.getValue())) {
return true;
}
}
return false;
}
}
Loading
Loading