This is an automated email from the ASF dual-hosted git repository.
adoroszlai 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 eb178061112 HDDS-14355. Allow createKey to skip allocateBlock for
empty key (#9626)
eb178061112 is described below
commit eb1780611125d083f6ca0a1047006bcc40e4736b
Author: Ivan Andika <[email protected]>
AuthorDate: Thu Jan 15 17:16:21 2026 +0800
HDDS-14355. Allow createKey to skip allocateBlock for empty key (#9626)
---
.../hadoop/hdds/scm/pipeline/PipelineManager.java | 5 +
.../hdds/scm/pipeline/PipelineManagerImpl.java | 5 +
.../hdds/scm/pipeline/SCMPipelineMetrics.java | 7 +
.../hdds/scm/pipeline/MockPipelineManager.java | 5 +
hadoop-ozone/integration-test-s3/pom.xml | 5 +
.../ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java | 27 ++-
.../ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java | 40 +++++
.../ozone/client/rpc/OzoneRpcClientTests.java | 52 ++++++
.../client/rpc/TestBlockDataStreamOutput.java | 4 +-
.../org/apache/hadoop/ozone/om/TestOmMetrics.java | 3 +
.../ozone/om/request/file/OMFileCreateRequest.java | 7 +-
.../ozone/om/request/key/OMKeyCreateRequest.java | 23 ++-
.../om/request/file/TestOMFileCreateRequest.java | 124 +++++++++++++
.../om/request/key/TestOMKeyCreateRequest.java | 196 ++++++++++++++++++---
14 files changed, 468 insertions(+), 35 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
index b4fe1559fd4..6a448d6c88d 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java
@@ -222,4 +222,9 @@ void reinitialize(Table<PipelineID, Pipeline> pipelineStore)
boolean hasEnoughSpace(Pipeline pipeline, long containerSize);
int openContainerLimit(List<DatanodeDetails> datanodes);
+
+ /**
+ * Get the pipeline metrics.
+ */
+ SCMPipelineMetrics getMetrics();
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java
index f53275c65b5..9c529e22e7e 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerImpl.java
@@ -944,4 +944,9 @@ public void acquireWriteLock() {
public void releaseWriteLock() {
lock.writeLock().unlock();
}
+
+ @Override
+ public SCMPipelineMetrics getMetrics() {
+ return metrics;
+ }
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java
index 3ba1df1a806..187d4be74b6 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/SCMPipelineMetrics.java
@@ -189,4 +189,11 @@ void incNumPipelineContainSameDatanodes() {
public void updatePipelineCreationLatencyNs(long startNanos) {
pipelineCreationLatencyNs.add(Time.monotonicNowNanos() - startNanos);
}
+
+ /**
+ * Return number of blocks allocated across all pipelines.
+ */
+ public long getTotalNumBlocksAllocated() {
+ return
numBlocksAllocated.values().stream().mapToLong(MutableCounterLong::value).sum();
+ }
}
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
index 04a1dc1bd4e..d6a3fc54635 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/MockPipelineManager.java
@@ -341,4 +341,9 @@ public int openContainerLimit(List<DatanodeDetails>
datanodes) {
// For tests that do not care about this limit, return a large value.
return Integer.MAX_VALUE;
}
+
+ @Override
+ public SCMPipelineMetrics getMetrics() {
+ return null;
+ }
}
diff --git a/hadoop-ozone/integration-test-s3/pom.xml
b/hadoop-ozone/integration-test-s3/pom.xml
index 46b3cbc6513..5d651dec31f 100644
--- a/hadoop-ozone/integration-test-s3/pom.xml
+++ b/hadoop-ozone/integration-test-s3/pom.xml
@@ -86,6 +86,11 @@
<artifactId>hdds-server-framework</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.ozone</groupId>
+ <artifactId>hdds-server-scm</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.ozone</groupId>
<artifactId>hdds-test-utils</artifactId>
diff --git
a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java
b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java
index c872d19f527..af890ff17e8 100644
---
a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java
+++
b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java
@@ -25,6 +25,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -109,6 +110,7 @@
import org.apache.hadoop.ozone.client.ObjectStore;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneKeyDetails;
import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
@@ -400,16 +402,39 @@ public void testPutDoubleSlashPrefixObject() throws
IOException {
}
@Test
- public void testPutObjectEmpty() {
+ public void testPutObjectEmpty() throws Exception {
final String bucketName = getBucketName();
final String keyName = getKeyName();
final String content = "";
s3Client.createBucket(bucketName);
+ long initialAllocatedBlocks =
+ cluster.getStorageContainerManager().getPipelineManager()
+ .getMetrics().getTotalNumBlocksAllocated();
+
InputStream is = new
ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
PutObjectResult putObjectResult = s3Client.putObject(bucketName, keyName,
is, new ObjectMetadata());
assertEquals("d41d8cd98f00b204e9800998ecf8427e",
putObjectResult.getETag());
+
+ // Verify via Ozone client that the key has no block locations
+ try (OzoneClient ozoneClient = cluster.newClient()) {
+ ObjectStore store = ozoneClient.getObjectStore();
+ OzoneVolume volume = store.getS3Volume();
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ OzoneKeyDetails keyDetails = bucket.getKey(keyName);
+ assertNotNull(keyDetails);
+ assertEquals(0, keyDetails.getDataSize(),
+ "Empty S3 object should have dataSize of 0");
+ assertTrue(keyDetails.getOzoneKeyLocations().isEmpty(),
+ "Empty S3 object should have no block locations");
+ }
+
+ // createKey should skip block allocation if data size is 0
+ long currentAllocatedBlocks =
+
cluster.getStorageContainerManager().getPipelineManager().getMetrics().getTotalNumBlocksAllocated();
+ assertEquals(initialAllocatedBlocks, currentAllocatedBlocks);
}
@Test
diff --git
a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java
b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java
index 73dac51346d..5a46afc435c 100644
---
a/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java
+++
b/hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java
@@ -25,6 +25,7 @@
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static software.amazon.awssdk.core.sync.RequestBody.fromString;
@@ -62,6 +63,7 @@
import org.apache.hadoop.ozone.client.ObjectStore;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneKeyDetails;
import org.apache.hadoop.ozone.client.OzoneVolume;
import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
import org.apache.hadoop.ozone.s3.S3ClientFactory;
@@ -233,6 +235,44 @@ public void testPutObject() {
assertEquals("\"37b51d194a7513e45b56f6524f2d51f2\"",
getObjectResponse.eTag());
}
+ @Test
+ public void testPutObjectEmpty() throws Exception {
+ final String bucketName = getBucketName();
+ final String keyName = getKeyName();
+ final String content = "";
+ s3Client.createBucket(b -> b.bucket(bucketName));
+
+ long initialAllocatedBlocks =
+ cluster.getStorageContainerManager().getPipelineManager()
+ .getMetrics().getTotalNumBlocksAllocated();
+
+ PutObjectResponse putObjectResponse = s3Client.putObject(b -> b
+ .bucket(bucketName)
+ .key(keyName),
+ RequestBody.fromString(content));
+
+ assertEquals("\"d41d8cd98f00b204e9800998ecf8427e\"",
putObjectResponse.eTag());
+
+ // Verify via Ozone client that the key has no block locations
+ try (OzoneClient ozoneClient = cluster.newClient()) {
+ ObjectStore store = ozoneClient.getObjectStore();
+ OzoneVolume volume = store.getS3Volume();
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ OzoneKeyDetails keyDetails = bucket.getKey(keyName);
+ assertNotNull(keyDetails);
+ assertEquals(0, keyDetails.getDataSize(),
+ "Empty S3 object should have dataSize of 0");
+ assertTrue(keyDetails.getOzoneKeyLocations().isEmpty(),
+ "Empty S3 object should have no block locations");
+ }
+
+ // createKey should skip block allocation if data size is 0
+ long currentAllocatedBlocks =
+
cluster.getStorageContainerManager().getPipelineManager().getMetrics().getTotalNumBlocksAllocated();
+ assertEquals(initialAllocatedBlocks, currentAllocatedBlocks);
+ }
+
@Test
public void testListObjectsMany() throws Exception {
testListObjectsMany(false);
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
index 3bc30fa0c42..dedc67fece3 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java
@@ -5271,4 +5271,56 @@ public void testGetObjectTagging(BucketLayout
bucketLayout) throws Exception {
assertEquals(tags.size(), tagsRetrieved.size());
assertThat(tagsRetrieved).containsAllEntriesOf(tags);
}
+
+ @Test
+ public void testCreateEmptyKeySkipBlockAllocation()
+ throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+
+ getStore().createVolume(volumeName);
+ OzoneVolume volume = getStore().getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ long initialAllocatedBlocks =
+ getCluster().getStorageContainerManager().getPipelineManager()
+ .getMetrics().getTotalNumBlocksAllocated();
+ // Don't write anything - this is an empty file
+ OzoneOutputStream out = bucket.createKey(keyName, 0);
+ out.close();
+
+ // createKey should skip block allocation if data size is 0
+ long currentAllocatedBlocks =
+
getCluster().getStorageContainerManager().getPipelineManager().getMetrics().getTotalNumBlocksAllocated();
+ assertEquals(initialAllocatedBlocks, currentAllocatedBlocks);
+ }
+
+ @Test
+ public void testCreateEmptyFileNotSkipBlockAllocation()
+ throws Exception {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+
+ getStore().createVolume(volumeName);
+ OzoneVolume volume = getStore().getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ long initialAllocatedBlocks =
+
getCluster().getStorageContainerManager().getPipelineManager().getMetrics().getTotalNumBlocksAllocated();
+ // Don't write anything - this is an empty file
+ OzoneOutputStream out = bucket.createFile(keyName, 0,
+ RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.ONE),
+ false,
+ false);
+ out.close();
+
+ // OM should call allocateBlock in OMFileCreateRequest regardless of data
size
+ long currentAllocatedBlocks =
+
getCluster().getStorageContainerManager().getPipelineManager().getMetrics().getTotalNumBlocksAllocated();
+ assertEquals(initialAllocatedBlocks + 1, currentAllocatedBlocks);
+ }
}
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestBlockDataStreamOutput.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestBlockDataStreamOutput.java
index 3fa28a0da36..3cea0590d85 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestBlockDataStreamOutput.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestBlockDataStreamOutput.java
@@ -318,7 +318,7 @@ public void testTotalAckDataLength(boolean flushDelay)
throws Exception {
int dataLength = 400;
String keyName = getKeyName();
OzoneDataStreamOutput key = createKey(
- client, keyName, 0);
+ client, keyName, dataLength);
byte[] data =
ContainerTestHelper.getFixedLengthString(keyString, dataLength)
.getBytes(UTF_8);
@@ -345,7 +345,7 @@ public void testDatanodeVersion(boolean flushDelay) throws
Exception {
}
String keyName = getKeyName();
- OzoneDataStreamOutput key = createKey(client, keyName, 0);
+ OzoneDataStreamOutput key = createKey(client, keyName, 1);
KeyDataStreamOutput keyDataStreamOutput = (KeyDataStreamOutput)
key.getByteBufStreamOutput();
BlockDataStreamOutputEntry stream =
keyDataStreamOutput.getStreamEntries().get(0);
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
index 495ff705607..9d3aedd4edc 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmMetrics.java
@@ -966,9 +966,11 @@ private void doKeyOps(OmKeyArgs keyArgs) {
private OmKeyArgs createKeyArgs(String volumeName, String bucketName,
ReplicationConfig repConfig) throws IOException {
+ int dataLength = 10;
OmKeyLocationInfo keyLocationInfo = new OmKeyLocationInfo.Builder()
.setBlockID(new BlockID(new ContainerBlockID(1, 1)))
.setPipeline(MockPipeline.createSingleNodePipeline())
+ .setLength(dataLength)
.build();
keyLocationInfo.setCreateVersion(0);
@@ -980,6 +982,7 @@ private OmKeyArgs createKeyArgs(String volumeName, String
bucketName,
.setKeyName(keyName)
.setAcls(Lists.emptyList())
.setReplicationConfig(repConfig)
+ .setDataSize(dataLength)
.setOwnerName(UserGroupInformation.getCurrentUser().getShortUserName())
.build();
}
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
index ecb86300835..9788cfbafe1 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
@@ -123,7 +123,12 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
// TODO: Here we are allocating block with out any check for
// bucket/key/volume or not and also with out any authorization checks.
-
+ // We also allocate block even if requestedSize is 0 because unlike
+ // CreateKey which is used for S3 use case where the requested dataSize is
known in advance
+ // and KeyArgs.dataSize is not going to be set for empty key
+ // File system client does not know the final file size in advance but use
0 as
+ // the placeholder for the data size. Therefore, we should at least
allocate a
+ // single block and we cannot simply skip the allocate block call
List< OmKeyLocationInfo > omKeyLocationInfoList =
allocateBlock(ozoneManager.getScmClient(),
ozoneManager.getBlockTokenSecretManager(), repConfig,
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
index 345ac492a04..d34320ecb8d 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
@@ -26,6 +26,7 @@
import java.nio.file.InvalidPathException;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -117,6 +118,7 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
CreateKeyRequest.Builder newCreateKeyRequest = null;
KeyArgs.Builder newKeyArgs = null;
+ UserInfo userInfo = getUserInfo();
if (!keyArgs.getIsMultipartKey()) {
long scmBlockSize = ozoneManager.getScmBlockSize();
@@ -143,10 +145,17 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
// bucket/key/volume or not and also with out any authorization checks.
// As for a client for the first time this can be executed on any OM,
// till leader is identified.
- UserInfo userInfo = getUserInfo();
- List<OmKeyLocationInfo> omKeyLocationInfoList =
- captureLatencyNs(perfMetrics.getCreateKeyAllocateBlockLatencyNs(),
- () -> allocateBlock(ozoneManager.getScmClient(),
+
+ List<OmKeyLocationInfo> omKeyLocationInfoList;
+ final long effectiveDataSize;
+ // Skip block allocation if dataSize <= 0. We also consider unspecified
dataSize as
+ // empty key since the client will not set dataSize if the key is empty
(i.e. dataSize <= 0),
+ if (!keyArgs.hasDataSize() || keyArgs.getDataSize() <= 0) {
+ omKeyLocationInfoList = Collections.emptyList();
+ effectiveDataSize = 0;
+ } else {
+ omKeyLocationInfoList =
captureLatencyNs(perfMetrics.getCreateKeyAllocateBlockLatencyNs(),
+ () -> allocateBlock(ozoneManager.getScmClient(),
ozoneManager.getBlockTokenSecretManager(), repConfig,
new ExcludeList(), requestedSize, scmBlockSize,
ozoneManager.getPreallocateBlocksMax(),
@@ -155,10 +164,12 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
ozoneManager.getMetrics(),
keyArgs.getSortDatanodes(),
userInfo));
+ effectiveDataSize = requestedSize;
+ }
newKeyArgs = keyArgs.toBuilder().setModificationTime(Time.now())
.setType(type).setFactor(factor)
- .setDataSize(requestedSize);
+ .setDataSize(effectiveDataSize);
newKeyArgs.addAllKeyLocations(omKeyLocationInfoList.stream()
.map(info -> info.getProtobuf(false,
@@ -186,7 +197,7 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
.setClientID(UniqueId.next());
return getOmRequest().toBuilder()
- .setCreateKeyRequest(newCreateKeyRequest).setUserInfo(getUserInfo())
+ .setCreateKeyRequest(newCreateKeyRequest).setUserInfo(userInfo)
.build();
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
index a5b792c717e..3004f511480 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java
@@ -33,7 +33,12 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import jakarta.annotation.Nonnull;
@@ -45,8 +50,14 @@
import org.apache.hadoop.crypto.CipherSuite;
import org.apache.hadoop.crypto.CryptoProtocolVersion;
import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension;
+import org.apache.hadoop.hdds.client.ContainerBlockID;
import org.apache.hadoop.hdds.client.ReplicationConfig;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.hadoop.ozone.om.helpers.BucketEncryptionKeyInfo;
@@ -689,6 +700,119 @@ protected OMRequest createFileRequest(
}
+ /**
+ * Test that SCM's allocateBlock is still called when creating an empty file.
+ */
+ @Test
+ public void testZeroSizedFileShouldCallAllocateBlock() throws Exception {
+ KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
+ .setVolumeName(volumeName).setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setFactor(HddsProtos.ReplicationFactor.ONE)
+ .setType(HddsProtos.ReplicationType.RATIS)
+ .setDataSize(0); // Set data size to 0 for empty file
+
+ CreateFileRequest createFileRequest = CreateFileRequest.newBuilder()
+ .setKeyArgs(keyArgs)
+ .setIsOverwrite(false)
+ .setIsRecursive(true).build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.CreateFile)
+ .setClientId(UUID.randomUUID().toString())
+ .setCreateFileRequest(createFileRequest).build();
+
+ OMFileCreateRequest omFileCreateRequest =
getOMFileCreateRequest(omRequest);
+
+ OMRequest modifiedOmRequest = omFileCreateRequest.preExecute(ozoneManager);
+
+ // Verify that SCM's allocateBlock is always called
+ verify(scmBlockLocationProtocol, atLeastOnce())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ // Verify key locations are present in the response
+ assertTrue(modifiedOmRequest.hasCreateFileRequest());
+ CreateFileRequest responseCreateFileRequest =
+ modifiedOmRequest.getCreateFileRequest();
+ assertTrue(
+ responseCreateFileRequest.getKeyArgs().getKeyLocationsCount() > 0,
+ "File with zero dataSize should still have key locations allocated");
+ }
+
+ /**
+ * Test that SCM's allocateBlock is still called when creating a file
+ * without explicitly setting dataSize (should use default scmBlockSize).
+ */
+ @Test
+ public void testFileWithoutDataSizeShouldAllocateBlock() throws Exception {
+ // Setup mock to return valid blocks
+ Pipeline pipeline = Pipeline.newBuilder()
+ .setState(Pipeline.PipelineState.OPEN)
+ .setId(PipelineID.randomId())
+ .setReplicationConfig(
+ StandaloneReplicationConfig.getInstance(
+ HddsProtos.ReplicationFactor.ONE))
+ .setNodes(new ArrayList<>())
+ .build();
+
+ AllocatedBlock.Builder blockBuilder = new AllocatedBlock.Builder()
+ .setPipeline(pipeline)
+ .setContainerBlockID(new ContainerBlockID(CONTAINER_ID, LOCAL_ID));
+
+ when(scmBlockLocationProtocol.allocateBlock(
+ anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString()))
+ .thenAnswer(invocation -> {
+ int num = invocation.getArgument(1);
+ List<AllocatedBlock> allocatedBlocks = new ArrayList<>(num);
+ for (int i = 0; i < num; i++) {
+ blockBuilder.setContainerBlockID(
+ new ContainerBlockID(CONTAINER_ID + i, LOCAL_ID + i));
+ allocatedBlocks.add(blockBuilder.build());
+ }
+ return allocatedBlocks;
+ });
+
+ // Create a file request WITHOUT setting dataSize (should default to
scmBlockSize)
+ KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
+ .setVolumeName(volumeName).setBucketName(bucketName)
+ .setKeyName(keyName)
+ .setFactor(HddsProtos.ReplicationFactor.ONE)
+ .setType(HddsProtos.ReplicationType.RATIS);
+ // Note: dataSize is not set
+
+ CreateFileRequest createFileRequest = CreateFileRequest.newBuilder()
+ .setKeyArgs(keyArgs)
+ .setIsOverwrite(false)
+ .setIsRecursive(true).build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.CreateFile)
+ .setClientId(UUID.randomUUID().toString())
+ .setCreateFileRequest(createFileRequest).build();
+
+ OMFileCreateRequest omFileCreateRequest =
getOMFileCreateRequest(omRequest);
+
+ OMRequest modifiedOmRequest = omFileCreateRequest.preExecute(ozoneManager);
+
+ // Verify that SCM's allocateBlock was called
+ verify(scmBlockLocationProtocol, atLeastOnce())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ // Verify key locations are present in the response
+ assertTrue(modifiedOmRequest.hasCreateFileRequest());
+ CreateFileRequest responseCreateFileRequest =
+ modifiedOmRequest.getCreateFileRequest();
+ assertTrue(
+ responseCreateFileRequest.getKeyArgs().getKeyLocationsCount() > 0,
+ "File without explicit dataSize should have key locations allocated");
+ }
+
/**
* Gets OMFileCreateRequest reference.
*
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
index b4cf5f7cc14..1666f4cb38e 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java
@@ -39,6 +39,12 @@
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.IOException;
@@ -52,11 +58,17 @@
import java.util.Map;
import java.util.UUID;
import java.util.stream.Collectors;
+import org.apache.hadoop.hdds.client.ContainerBlockID;
import org.apache.hadoop.hdds.client.ECReplicationConfig;
import org.apache.hadoop.hdds.client.RatisReplicationConfig;
import org.apache.hadoop.hdds.client.ReplicationConfig;
+import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.KeyValue;
+import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock;
+import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList;
+import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
+import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.om.OmConfig;
@@ -275,13 +287,27 @@ private void checkResponse(
assertNotNull(omKeyInfo);
assertNotNull(omKeyInfo.getLatestVersionLocations());
- // As our data size is 100, and scmBlockSize is default to 1000, so we
- // shall have only one block.
List<OmKeyLocationInfo> omKeyLocationInfoList =
omKeyInfo.getLatestVersionLocations().getLocationList();
- assertEquals(1, omKeyLocationInfoList.size());
+ if (modifiedOmRequest.getCreateKeyRequest().getKeyArgs().getDataSize() >
0) {
+ // As our data size is 100, and scmBlockSize is default to 1000, so we
+ // shall have only one block.
+ assertEquals(1, omKeyLocationInfoList.size());
+
+ OmKeyLocationInfo omKeyLocationInfo = omKeyLocationInfoList.get(0);
+ // Check data of the block
+ OzoneManagerProtocolProtos.KeyLocation keyLocation =
+
modifiedOmRequest.getCreateKeyRequest().getKeyArgs().getKeyLocations(0);
+
+ assertEquals(keyLocation.getBlockID().getContainerBlockID()
+ .getContainerID(), omKeyLocationInfo.getContainerID());
+ assertEquals(keyLocation.getBlockID().getContainerBlockID()
+ .getLocalID(), omKeyLocationInfo.getLocalID());
+ } else {
+ // When creating an empty key, there should not be any blocks
+ assertEquals(0, omKeyLocationInfoList.size());
+ }
- OmKeyLocationInfo omKeyLocationInfo = omKeyLocationInfoList.get(0);
// Check modification time
assertEquals(modifiedOmRequest.getCreateKeyRequest()
@@ -294,15 +320,6 @@ private void checkResponse(
assertNotEquals(omKeyInfo.getModificationTime(),
omKeyInfo.getCreationTime());
}
-
- // Check data of the block
- OzoneManagerProtocolProtos.KeyLocation keyLocation =
-
modifiedOmRequest.getCreateKeyRequest().getKeyArgs().getKeyLocations(0);
-
- assertEquals(keyLocation.getBlockID().getContainerBlockID()
- .getContainerID(), omKeyLocationInfo.getContainerID());
- assertEquals(keyLocation.getBlockID().getContainerBlockID()
- .getLocalID(), omKeyLocationInfo.getLocalID());
}
@ParameterizedTest
@@ -633,7 +650,8 @@ private OMRequest doPreExecute(OMRequest originalOMRequest)
throws Exception {
keyArgs.getEcReplicationConfig().getData() : 1;
long blockSize = ozoneManager.getScmBlockSize();
long preAllocatedBlocks = Math.min(ozoneManager.getPreallocateBlocksMax(),
- (keyArgs.getDataSize() - 1) / (blockSize * dataGroupSize) + 1);
+ keyArgs.getDataSize() > 0 ?
+ (keyArgs.getDataSize() - 1) / (blockSize * dataGroupSize) + 1 : 0);
// Time should be set
assertThat(keyArgs.getModificationTime()).isGreaterThan(0);
@@ -651,17 +669,19 @@ private OMRequest doPreExecute(OMRequest
originalOMRequest) throws Exception {
keyArgs.getKeyLocationsList();
// KeyLocation should be set.
assertEquals(preAllocatedBlocks, keyLocations.size());
- assertEquals(CONTAINER_ID,
- keyLocations.get(0).getBlockID().getContainerBlockID()
- .getContainerID());
- assertEquals(LOCAL_ID,
- keyLocations.get(0).getBlockID().getContainerBlockID()
- .getLocalID());
- assertTrue(keyLocations.get(0).hasPipeline());
-
- assertEquals(0, keyLocations.get(0).getOffset());
-
- assertEquals(scmBlockSize, keyLocations.get(0).getLength());
+ if (preAllocatedBlocks > 0) {
+ assertEquals(CONTAINER_ID,
+ keyLocations.get(0).getBlockID().getContainerBlockID()
+ .getContainerID());
+ assertEquals(LOCAL_ID,
+ keyLocations.get(0).getBlockID().getContainerBlockID()
+ .getLocalID());
+ assertTrue(keyLocations.get(0).hasPipeline());
+
+ assertEquals(0, keyLocations.get(0).getOffset());
+
+ assertEquals(scmBlockSize, keyLocations.get(0).getLength());
+ }
} else {
// We don't create blocks for multipart key in createKey preExecute.
assertEquals(0, keyArgs.getKeyLocationsList().size());
@@ -1070,6 +1090,132 @@ private void verifyKeyInheritAcls(List<OzoneAcl>
keyAcls,
assertThat(keyAcls).doesNotContain(parentAccessAcl);
}
+ /**
+ * Test that SCM's allocateBlock is not called when creating an empty key.
+ */
+ @Test
+ public void testEmptyKeyKeyDoesNotCallScmAllocateBlock() throws Exception {
+ KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
+ .setVolumeName(volumeName).setBucketName(bucketName)
+ .setKeyName(keyName).setIsMultipartKey(false)
+ .setFactor(
+ ((RatisReplicationConfig)
replicationConfig).getReplicationFactor())
+ .setType(replicationConfig.getReplicationType())
+ .setLatestVersionLocation(true)
+ .setDataSize(0); // explicitly set data size to 0
+
+ CreateKeyRequest createKeyRequest =
+ CreateKeyRequest.newBuilder().setKeyArgs(keyArgs).build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
+ .setClientId(UUID.randomUUID().toString())
+ .setCreateKeyRequest(createKeyRequest).build();
+
+ OMKeyCreateRequest omKeyCreateRequest = getOMKeyCreateRequest(omRequest);
+
+ OMRequest modifiedOmRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+ // Verify that SCM's allocateBlock was never called
+ verify(scmBlockLocationProtocol, never())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ verify(scmBlockLocationProtocol, never())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ assertTrue(modifiedOmRequest.hasCreateKeyRequest());
+ CreateKeyRequest responseCreateKeyRequest =
+ modifiedOmRequest.getCreateKeyRequest();
+ assertEquals(0,
+ responseCreateKeyRequest.getKeyArgs().getKeyLocationsCount(),
+ "Empty key should have no key locations");
+
+ assertEquals(0,
+ responseCreateKeyRequest.getKeyArgs().getDataSize(),
+ "Empty key should have dataSize of 0");
+ }
+
+ /**
+ * Test that SCM's allocateBlock is not called when creating a key without
specifying the dataSize.
+ */
+ @Test
+ public void testKeyWithoutDataSizeCallsScmAllocateBlock() throws Exception {
+ // Setup mock to return valid blocks
+ Pipeline pipeline = Pipeline.newBuilder()
+ .setState(Pipeline.PipelineState.OPEN)
+ .setId(PipelineID.randomId())
+ .setReplicationConfig(
+ StandaloneReplicationConfig.getInstance(
+
org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE))
+ .setNodes(new ArrayList<>())
+ .build();
+
+ AllocatedBlock.Builder blockBuilder = new AllocatedBlock.Builder()
+ .setPipeline(pipeline)
+ .setContainerBlockID(new ContainerBlockID(CONTAINER_ID, LOCAL_ID));
+
+ when(scmBlockLocationProtocol.allocateBlock(
+ anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString()))
+ .thenAnswer(invocation -> {
+ int num = invocation.getArgument(1);
+ List<AllocatedBlock> allocatedBlocks = new ArrayList<>(num);
+ for (int i = 0; i < num; i++) {
+ blockBuilder.setContainerBlockID(
+ new ContainerBlockID(CONTAINER_ID + i, LOCAL_ID + i));
+ allocatedBlocks.add(blockBuilder.build());
+ }
+ return allocatedBlocks;
+ });
+
+ // Create a key request without setting dataSize (should default to
scmBlockSize)
+ KeyArgs.Builder keyArgs = KeyArgs.newBuilder()
+ .setVolumeName(volumeName).setBucketName(bucketName)
+ .setKeyName(keyName).setIsMultipartKey(false)
+ .setFactor(
+ ((RatisReplicationConfig)
replicationConfig).getReplicationFactor())
+ .setType(replicationConfig.getReplicationType())
+ .setLatestVersionLocation(true);
+
+ CreateKeyRequest createKeyRequest =
+ CreateKeyRequest.newBuilder().setKeyArgs(keyArgs).build();
+
+ OMRequest omRequest = OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.CreateKey)
+ .setClientId(UUID.randomUUID().toString())
+ .setCreateKeyRequest(createKeyRequest).build();
+
+ OMKeyCreateRequest omKeyCreateRequest = getOMKeyCreateRequest(omRequest);
+
+ OMRequest modifiedOmRequest = omKeyCreateRequest.preExecute(ozoneManager);
+
+ verify(scmBlockLocationProtocol, never())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ verify(scmBlockLocationProtocol, never())
+ .allocateBlock(anyLong(), anyInt(),
+ any(ReplicationConfig.class), anyString(),
+ any(ExcludeList.class), anyString());
+
+ assertTrue(modifiedOmRequest.hasCreateKeyRequest());
+ CreateKeyRequest responseCreateKeyRequest =
+ modifiedOmRequest.getCreateKeyRequest();
+ assertEquals(0,
+ responseCreateKeyRequest.getKeyArgs().getKeyLocationsCount(),
+ "Empty key should have no key locations");
+
+ assertEquals(0,
+ responseCreateKeyRequest.getKeyArgs().getDataSize(),
+ "Empty key should have dataSize of 0");
+ }
+
protected void addToKeyTable(String keyName) throws Exception {
OMRequestTestUtils.addKeyToTable(false, volumeName, bucketName,
keyName.substring(1), 0L, RatisReplicationConfig.getInstance(THREE),
omMetadataManager);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
