From a29de015037404d6bbf7c31cb63f2f9fe65dfdb7 Mon Sep 17 00:00:00 2001 From: sumitagrawl Date: Wed, 2 Aug 2023 16:41:14 +0530 Subject: [PATCH 1/5] HDDS-9110. Bucket owner is getting unset on setting quota on that bucket --- .../bucket/OMBucketSetPropertyRequest.java | 44 ++----------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index 912289f8434f..dbbd6c7ee4f2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -146,15 +146,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OMException.ResultCodes.NOT_SUPPORTED_OPERATION); } - OmBucketInfo.Builder bucketInfoBuilder = OmBucketInfo.newBuilder(); - bucketInfoBuilder.setVolumeName(dbBucketInfo.getVolumeName()) - .setBucketName(dbBucketInfo.getBucketName()) - .setObjectID(dbBucketInfo.getObjectID()) - .setBucketLayout(dbBucketInfo.getBucketLayout()) - .setBucketEncryptionKey(dbBucketInfo.getEncryptionKeyInfo()) - .setUpdateID(transactionLogIndex); + OmBucketInfo.Builder bucketInfoBuilder = dbBucketInfo.toBuilder(); + bucketInfoBuilder.setUpdateID(transactionLogIndex); bucketInfoBuilder.addAllMetadata(KeyValueUtil .getFromProtobuf(bucketArgs.getMetadataList())); + bucketInfoBuilder.setModificationTime( + setBucketPropertyRequest.getModificationTime()); //Check StorageType to update StorageType storageType = omBucketArgs.getStorageType(); @@ -162,8 +159,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, bucketInfoBuilder.setStorageType(storageType); LOG.debug("Updating bucket storage type for bucket: {} in volume: {}", bucketName, volumeName); - } else { - bucketInfoBuilder.setStorageType(dbBucketInfo.getStorageType()); } //Check Versioning to update @@ -172,9 +167,6 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, bucketInfoBuilder.setIsVersionEnabled(versioning); LOG.debug("Updating bucket versioning for bucket: {} in volume: {}", bucketName, volumeName); - } else { - bucketInfoBuilder - .setIsVersionEnabled(dbBucketInfo.getIsVersionEnabled()); } //Check quotaInBytes and quotaInNamespace to update @@ -184,15 +176,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (checkQuotaBytesValid(omMetadataManager, omVolumeArgs, omBucketArgs, dbBucketInfo)) { bucketInfoBuilder.setQuotaInBytes(omBucketArgs.getQuotaInBytes()); - } else { - bucketInfoBuilder.setQuotaInBytes(dbBucketInfo.getQuotaInBytes()); } if (checkQuotaNamespaceValid(omVolumeArgs, omBucketArgs, dbBucketInfo)) { bucketInfoBuilder.setQuotaInNamespace( omBucketArgs.getQuotaInNamespace()); - } else { - bucketInfoBuilder.setQuotaInNamespace( - dbBucketInfo.getQuotaInNamespace()); } DefaultReplicationConfig defaultReplicationConfig = @@ -200,31 +187,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, if (defaultReplicationConfig != null) { // Resetting the default replication config. bucketInfoBuilder.setDefaultReplicationConfig(defaultReplicationConfig); - } else if (dbBucketInfo.getDefaultReplicationConfig() != null) { - // Retaining existing default replication config - bucketInfoBuilder.setDefaultReplicationConfig( - dbBucketInfo.getDefaultReplicationConfig()); - } - - bucketInfoBuilder.setCreationTime(dbBucketInfo.getCreationTime()); - bucketInfoBuilder.setModificationTime( - setBucketPropertyRequest.getModificationTime()); - // Set acls from dbBucketInfo if it has any. - if (dbBucketInfo.getAcls() != null) { - bucketInfoBuilder.setAcls(dbBucketInfo.getAcls()); - } - - // Set the objectID to dbBucketInfo objectID, if present - if (dbBucketInfo.getObjectID() != 0) { - bucketInfoBuilder.setObjectID(dbBucketInfo.getObjectID()); } - // Set the updateID to current transaction log index - bucketInfoBuilder.setUpdateID(transactionLogIndex); - // Quota used remains unchanged - bucketInfoBuilder.setUsedBytes(dbBucketInfo.getUsedBytes()); - bucketInfoBuilder.setUsedNamespace(dbBucketInfo.getUsedNamespace()); - omBucketInfo = bucketInfoBuilder.build(); // Update table cache. From 1c30c9abf53f2d4382512ac3798de3f7c546dba4 Mon Sep 17 00:00:00 2001 From: Sumit Agrawal Date: Thu, 4 Jan 2024 22:01:08 +0530 Subject: [PATCH 2/5] HDDS-10061. NPE when container is loaded with missing container DB --- .../ozone/container/keyvalue/helpers/KeyValueContainerUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index f47d17d73883..6449832c7ea1 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -232,7 +232,7 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, LOG.error("Container DB file is missing for ContainerID {}. " + "Skipping loading of this container.", containerID); // Don't further process this container, as it is missing db file. - return; + throw new IOException("Container DB file is missing"); } kvContainerData.setDbFile(dbFile); From 876ab2dee88a34cb6bb98e93ef1b0dbf1a326a54 Mon Sep 17 00:00:00 2001 From: Sumit Agrawal Date: Fri, 5 Jan 2024 13:06:37 +0530 Subject: [PATCH 3/5] HDDS-9342. OM restart failed due to transactionLogIndex smaller than current updateID --- .../ozoneimpl/TestContainerReader.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index 3535e6e3c7b9..597d9e9824f7 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -45,6 +45,8 @@ import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; +import org.apache.ozone.test.GenericTestUtils; +import org.apache.ratis.util.FileUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.io.TempDir; @@ -56,6 +58,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import org.slf4j.LoggerFactory; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.DELETED; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING; @@ -308,6 +311,56 @@ public void testContainerReaderWithLoadException( assertEquals(containerCount - 1, containerSet1.containerCount()); } + @ContainerTestVersionInfo.ContainerTest + public void testContainerReaderWithRemovindDbPath( + ContainerTestVersionInfo versionInfo) throws Exception { + setLayoutAndSchemaVersion(versionInfo); + setup(versionInfo); + MutableVolumeSet volumeSet1; + HddsVolume hddsVolume1; + ContainerSet containerSet1 = new ContainerSet(1000); + File volumeDir1 = + Files.createDirectory(tempDir.resolve("volumeDirDbDelete")).toFile(); + RoundRobinVolumeChoosingPolicy volumeChoosingPolicy1; + + volumeSet1 = mock(MutableVolumeSet.class); + UUID datanode = UUID.randomUUID(); + hddsVolume1 = new HddsVolume.Builder(volumeDir1 + .getAbsolutePath()).conf(conf).datanodeUuid(datanode + .toString()).clusterID(clusterId).build(); + StorageVolumeUtil.checkVolume(hddsVolume1, clusterId, clusterId, conf, + null, null); + volumeChoosingPolicy1 = mock(RoundRobinVolumeChoosingPolicy.class); + when(volumeChoosingPolicy1.chooseVolume(anyList(), anyLong())) + .thenReturn(hddsVolume1); + + List dbPathList = new ArrayList<>(); + int containerCount = 3; + for (int i = 0; i < containerCount; i++) { + KeyValueContainerData keyValueContainerData = new KeyValueContainerData(i, + layout, + (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), + datanodeId.toString()); + KeyValueContainer keyValueContainer = + new KeyValueContainer(keyValueContainerData, conf); + keyValueContainer.create(volumeSet1, volumeChoosingPolicy1, clusterId); + dbPathList.add(keyValueContainerData.getDbFile()); + } + ContainerCache.getInstance(conf).shutdownCache(); + for (File dbPath : dbPathList) { + FileUtils.deleteFully(dbPath.toPath()); + } + + GenericTestUtils.LogCapturer dnLogs = GenericTestUtils.LogCapturer.captureLogs( + LoggerFactory.getLogger(ContainerReader.class)); + dnLogs.clearOutput(); + ContainerReader containerReader = new ContainerReader(volumeSet1, + hddsVolume1, containerSet1, conf, true); + containerReader.readVolume(hddsVolume1.getHddsRootDir()); + assertEquals(0, containerSet1.containerCount()); + assertTrue(dnLogs.getOutput().contains("Container DB file is missing")); + } + @ContainerTestVersionInfo.ContainerTest public void testMultipleContainerReader(ContainerTestVersionInfo versionInfo) throws Exception { From fdf13cb44d527ef017e589f5ce32a0ddf54e1a68 Mon Sep 17 00:00:00 2001 From: Sumit Agrawal Date: Fri, 5 Jan 2024 13:09:20 +0530 Subject: [PATCH 4/5] review comment fix --- .../container/keyvalue/helpers/KeyValueContainerUtil.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index 6449832c7ea1..16847d1157c5 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -232,7 +232,8 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, LOG.error("Container DB file is missing for ContainerID {}. " + "Skipping loading of this container.", containerID); // Don't further process this container, as it is missing db file. - throw new IOException("Container DB file is missing"); + throw new IOException("Container DB file is missing for containerID " + + containerID); } kvContainerData.setDbFile(dbFile); From c43cb541484386f41b580ef9e6bb192cfa3d2879 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" <6454655+adoroszlai@users.noreply.github.com> Date: Fri, 5 Jan 2024 22:10:01 +0100 Subject: [PATCH 5/5] Fix typo in test name Co-authored-by: Ethan Rose <33912936+errose28@users.noreply.github.com> --- .../hadoop/ozone/container/ozoneimpl/TestContainerReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index 597d9e9824f7..d48f0d3314e0 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -312,7 +312,7 @@ public void testContainerReaderWithLoadException( } @ContainerTestVersionInfo.ContainerTest - public void testContainerReaderWithRemovindDbPath( + public void testContainerReaderWithInvalidDbPath( ContainerTestVersionInfo versionInfo) throws Exception { setLayoutAndSchemaVersion(versionInfo); setup(versionInfo);