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 0f5b59089b HDDS-12691. Calculation of committed space in Datanode
seems incorrect (#8228)
0f5b59089b is described below
commit 0f5b59089b865a5d7a9fd305573e3a818c3836b9
Author: Siddhant Sangwan <[email protected]>
AuthorDate: Mon Apr 7 10:19:18 2025 +0530
HDDS-12691. Calculation of committed space in Datanode seems incorrect
(#8228)
---
.../ozone/container/common/impl/ContainerData.java | 23 ++-
.../common/impl/TestContainerPersistence.java | 155 ++++++++++++++++++++-
2 files changed, 167 insertions(+), 11 deletions(-)
diff --git
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
index 12c618b226..47ca8f1cc0 100644
---
a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
+++
b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
@@ -29,6 +29,7 @@
import static org.apache.hadoop.ozone.OzoneConsts.STATE;
import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import jakarta.annotation.Nullable;
@@ -226,6 +227,11 @@ public synchronized void setState(ContainerDataProto.State
state) {
}
}
+ @VisibleForTesting
+ void setCommittedSpace(boolean committedSpace) {
+ this.committedSpace = committedSpace;
+ }
+
/**
* Return's maximum size of the container in bytes.
* @return maxSize in bytes
@@ -431,8 +437,6 @@ public long getWriteBytes() {
* @param bytes the number of bytes write into the container.
*/
public void incrWriteBytes(long bytes) {
- long unused = getMaxSize() - getBytesUsed();
-
this.writeBytes.addAndGet(bytes);
/*
Increase the cached Used Space in VolumeInfo as it
@@ -440,11 +444,16 @@ public void incrWriteBytes(long bytes) {
periodically to update the Used Space in VolumeInfo.
*/
this.getVolume().incrementUsedSpace(bytes);
- // only if container size < max size
- if (committedSpace && unused > 0) {
- //with this write, container size might breach max size
- long decrement = Math.min(bytes, unused);
- this.getVolume().incCommittedBytes(0 - decrement);
+ // Calculate bytes used before this write operation.
+ // Note that getBytesUsed() already includes the 'bytes' from the current
write.
+ long bytesUsedBeforeWrite = getBytesUsed() - bytes;
+ // Calculate how much space was available within the max size limit before
this write
+ long availableSpaceBeforeWrite = getMaxSize() - bytesUsedBeforeWrite;
+ if (committedSpace && availableSpaceBeforeWrite > 0) {
+ // Decrement committed space only by the portion of the write that fits
within the originally committed space,
+ // up to maxSize
+ long decrement = Math.min(bytes, availableSpaceBeforeWrite);
+ this.getVolume().incCommittedBytes(-decrement);
}
}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
index 0a31b9746d..ec3e65345c 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java
@@ -627,13 +627,160 @@ public void testWriteChunk(ContainerTestVersionInfo
versionInfo)
}
/**
- * Writes many chunks of the same block into different chunk files and
- * verifies that we have that data in many files.
+ * Tests that committed space is correctly decremented when a write fits
entirely within the available space under
+ * max container size. It should be decremented by the number of bytes
written.
*
- * @throws IOException
- * @throws NoSuchAlgorithmException
+ * Before write: Container size is less than max size
+ * After write: Container size is still less than max size
+ */
+ @ContainerTestVersionInfo.ContainerTest
+ public void
testIncrWriteBytesDecrementsCommittedWhenWriteFits(ContainerTestVersionInfo
versionInfo)
+ throws IOException {
+ initSchemaAndVersionInfo(versionInfo);
+ BlockID blockID = ContainerTestHelper.
+ getTestBlockID(getTestContainerID());
+ long testContainerID = blockID.getContainerID();
+ Container container = containerSet.getContainer(testContainerID);
+ if (container == null) {
+ container = addContainer(containerSet, testContainerID);
+ }
+
+ long commitBytesBefore =
container.getContainerData().getVolume().getCommittedBytes();
+ long writeBytes = 256 * OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+ long commitBytesAfter =
container.getContainerData().getVolume().getCommittedBytes();
+ long commitDecrement = commitBytesBefore - commitBytesAfter;
+ // did we decrement commit bytes by the amount of data we wrote?
+ assertEquals(writeBytes, commitDecrement);
+ }
+
+ /**
+ * Tests the scenario where a write operation causes the container usage to
exceed max container size.
+ * The committed space should only be decremented by the amount of the write
that fit within the max limit.
+ *
+ * Before Write: Container size is within max size
+ * After Write: Container size exceeds max size
+ */
+ @ContainerTestVersionInfo.ContainerTest
+ public void
testIncrWriteBytesDecrementsCommittedCorrectlyWhenWriteExceedsMax(ContainerTestVersionInfo
versionInfo)
+ throws IOException {
+ initSchemaAndVersionInfo(versionInfo);
+ BlockID blockID = ContainerTestHelper.
+ getTestBlockID(getTestContainerID());
+ long testContainerID = blockID.getContainerID();
+ Container container = containerSet.getContainer(testContainerID);
+ if (container == null) {
+ container = addContainer(containerSet, testContainerID);
+ }
+
+ // fill the container close to the max size first
+ long writeBytes = container.getContainerData().getMaxSize() - 100;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+
+ // the next write will make the container size exceed max size
+ long commitBytesBefore =
container.getContainerData().getVolume().getCommittedBytes();
+ writeBytes = 256 * OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+ long commitBytesAfter =
container.getContainerData().getVolume().getCommittedBytes();
+ long commitDecrement = commitBytesBefore - commitBytesAfter;
+ // did we decrement commit bytes by the amount that was remaining, ie, 100
bytes?
+ assertEquals(100, commitDecrement);
+ }
+
+ /**
+ * Tests that committed space is not decremented if the container was already
+ * full (or overfull) before the write operation.
+ */
+ @ContainerTestVersionInfo.ContainerTest
+ public void
testIncrWriteBytesDoesNotDecrementCommittedWhenContainerFull(ContainerTestVersionInfo
versionInfo)
+ throws IOException {
+ initSchemaAndVersionInfo(versionInfo);
+ BlockID blockID = ContainerTestHelper.
+ getTestBlockID(getTestContainerID());
+ long testContainerID = blockID.getContainerID();
+ Container container = containerSet.getContainer(testContainerID);
+ if (container == null) {
+ container = addContainer(containerSet, testContainerID);
+ }
+
+ // fill the container completely first
+ long writeBytes = container.getContainerData().getMaxSize();
+ container.getContainerData().updateWriteStats(writeBytes, false);
+
+ // the next write will make the container size exceed max size
+ long commitBytesBefore =
container.getContainerData().getVolume().getCommittedBytes();
+ writeBytes = 256 * OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+ long commitBytesAfter =
container.getContainerData().getVolume().getCommittedBytes();
+ long commitDecrement = commitBytesBefore - commitBytesAfter;
+ // decrement should be 0, as the container was already full before the
write
+ assertEquals(0, commitDecrement);
+ }
+
+ /**
+ * In this test, the write fills the container exactly to max size.
Committed space should be decremented by the
+ * amount of bytes written.
*/
@ContainerTestVersionInfo.ContainerTest
+ public void
testIncrWriteBytesDecrementsCommittedCorrectlyWhenWrittenToBrim(ContainerTestVersionInfo
versionInfo)
+ throws IOException {
+ initSchemaAndVersionInfo(versionInfo);
+ BlockID blockID = ContainerTestHelper.
+ getTestBlockID(getTestContainerID());
+ long testContainerID = blockID.getContainerID();
+ Container container = containerSet.getContainer(testContainerID);
+ if (container == null) {
+ container = addContainer(containerSet, testContainerID);
+ }
+
+ // fill the container completely first
+ long writeBytes = container.getContainerData().getMaxSize() - 256 *
OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+
+ // the next write will make the container size exactly equal to max size
+ long commitBytesBefore =
container.getContainerData().getVolume().getCommittedBytes();
+ writeBytes = 256 * OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+ long commitBytesAfter =
container.getContainerData().getVolume().getCommittedBytes();
+ long commitDecrement = commitBytesBefore - commitBytesAfter;
+ // decrement should be writeBytes, as the write does not exceed max size
+ assertEquals(writeBytes, commitDecrement);
+ }
+
+ /**
+ * Commited space should not change when committedSpace is set to false.
+ */
+ @ContainerTestVersionInfo.ContainerTest
+ public void
testIncrWriteBytesDoesNotChangeCommittedSpaceWhenItsDisabled(ContainerTestVersionInfo
versionInfo)
+ throws IOException {
+ initSchemaAndVersionInfo(versionInfo);
+ BlockID blockID = ContainerTestHelper.
+ getTestBlockID(getTestContainerID());
+ long testContainerID = blockID.getContainerID();
+ Container container = containerSet.getContainer(testContainerID);
+ if (container == null) {
+ container = addContainer(containerSet, testContainerID);
+ }
+
+ // For setup, we need to set committedSpace to false first
+ container.getContainerData().setCommittedSpace(false);
+ long commitBytesBefore =
container.getContainerData().getVolume().getCommittedBytes();
+ long writeBytes = 256 * OzoneConsts.MB;
+ container.getContainerData().updateWriteStats(writeBytes, false);
+ long commitBytesAfter =
container.getContainerData().getVolume().getCommittedBytes();
+ long commitDecrement = commitBytesBefore - commitBytesAfter;
+ // decrement should be 0, as commit space is disabled for this container
+ assertEquals(0, commitDecrement);
+ }
+
+ /**
+ * Writes many chunks of the same block into different chunk files and
+ * verifies that we have that data in many files.
+ *
+ * @throws IOException
+ * @throws NoSuchAlgorithmException
+ */
+ @ContainerTestVersionInfo.ContainerTest
public void testWritReadManyChunks(ContainerTestVersionInfo versionInfo)
throws IOException {
initSchemaAndVersionInfo(versionInfo);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
