(ozone) branch master updated: HDDS-17024. Failed .container write during mark for close rolls the container back to OPEN without its space reservation (#11622)

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chungen0126 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 f39f7ce8b4c HDDS-17024. Failed .container write during mark for close rolls the container back to OPEN without its space reservation (#11622)
f39f7ce8b4c is described below

commit f39f7ce8b4c08b40c868332d60b982b29cb0e5a1
Author: Gargi Jaiswal <13...@users.noreply.github.com>
AuthorDate: Thu Oct 22 16:38:47 2026 +0530

    HDDS-17024. Failed .container write during mark for close rolls the container back to OPEN without its space reservation (#11622)
---
 .../container/keyvalue/KeyValueContainer.java      | 18 +++++++++++---
 .../container/keyvalue/TestKeyValueContainer.java  | 28 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
index 06a73b33373..fd016f3078b 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
@@ -480,16 +480,24 @@ private void updateContainerState(State newState) throws StorageContainerExcepti
   }
 
   /**
-   *
+   * Applies {@code update} to the in-memory container data and persists it to the .container file.
    * Must be invoked with the writeLock held.
+   * <p>
+   * The rollback only covers a failed write of the .container file: {@code update} is a {@link Runnable}, so it
+   * cannot throw a {@link StorageContainerException}, and only {@code updateContainerFile} can. When the write
+   * fails, the in-memory change has already been applied, so the previous state is restored to match the file on
+   * disk, unless the container became UNHEALTHY in the meantime. Leaving OPEN releases the space reserved for the
+   * container, and restoring OPEN does not reserve it again, so the reservation
+   * is restored explicitly.
    *
-   * @param update
-   * @throws StorageContainerException
+   * @param update the change to apply to the in-memory container data
+   * @throws StorageContainerException if the .container file cannot be written
    */
   private void updateContainerData(Runnable update)
       throws StorageContainerException {
     Preconditions.checkState(hasWriteLock());
     final State oldState = containerData.getState();
+    final boolean committedSpace = containerData.isCommittedSpace();
     try {
       update.run();
       // update the new container data to .container File
@@ -500,6 +508,10 @@ private void updateContainerData(Runnable update)
         // Failed to update .container file. Reset the state to old state only
         // if the current state is not unhealthy.
         containerData.setState(oldState);
+        // Leaving the old state may have released the reserved space, reserve it again.
+        if (committedSpace && !containerData.isCommittedSpace()) {
+          containerData.commitSpace();
+        }
       }
       throw ex;
     }
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
index 0e64de71f74..13b714df5e9 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
@@ -846,6 +846,34 @@ public void testCloseContainer(ContainerTestVersionInfo versionInfo)
         keyValueContainerData.getState());
   }
 
+  @ContainerTestVersionInfo.ContainerTest
+  public void testReservationKeptWhenMarkForCloseFails(ContainerTestVersionInfo versionInfo) throws Exception {
+    init(versionInfo);
+    createContainer();
+    // Reserve the space as the volume choosing policy does on create.
+    HddsVolume volume = keyValueContainerData.getVolume();
+    volume.incCommittedBytes(keyValueContainerData.getMaxSize());
+
+    // The .container file cannot be updated, so marking for close fails and rolls back to OPEN.
+    File metadataDir = new File(keyValueContainerData.getMetadataPath());
+    assertThat(metadataDir.setWritable(false)).isTrue();
+    try {
+      StorageContainerException ex = assertThrows(StorageContainerException.class,
+          () -> keyValueContainer.markContainerForClose());
+      assertThat(ex.getResult()).isEqualTo(ContainerProtos.Result.CONTAINER_FILES_CREATE_ERROR);
+    } finally {
+      assertThat(metadataDir.setWritable(true)).isTrue();
+    }
+    assertThat(keyValueContainerData.getState()).isEqualTo(ContainerProtos.ContainerDataProto.State.OPEN);
+    assertThat(keyValueContainerData.isCommittedSpace()).isTrue();
+    assertThat(volume.getCommittedBytes()).isEqualTo(keyValueContainerData.getMaxSize());
+
+    // Once the file can be written, marking for close succeeds and releases the space.
+    keyValueContainer.markContainerForClose();
+    assertThat(keyValueContainerData.getState()).isEqualTo(ContainerProtos.ContainerDataProto.State.CLOSING);
+    assertThat(volume.getCommittedBytes()).isZero();
+  }
+
   @ContainerTestVersionInfo.ContainerTest
   public void testReportOfUnhealthyContainer(
       ContainerTestVersionInfo versionInfo) throws Exception {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org