You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xichen01 (via GitHub)" <gi...@apache.org> on 2026/05/05 12:34:21 UTC
[PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
xichen01 opened a new pull request, #10191:
URL: https://github.com/apache/ozone/pull/10191
## What changes were proposed in this pull request?
Introduce StorageClass and storage policy, introducing storage policy related definitions
- Releated doc and discuss: https://github.com/apache/ozone/pull/6989
- A preview version for the full "Ozone Storage Policy" feature: https://github.com/ivandika3/ozone/tree/refs/heads/backport-storage-policy-storage-class
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15175
## How was this patch tested?
unit test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "greenwich (via GitHub)" <gi...@apache.org>.
greenwich commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3192150435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
Review Comment:
Do you think we may introduce the NVME storage type (here and below where it's needed)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "greenwich (via GitHub)" <gi...@apache.org>.
greenwich commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3192150435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
Review Comment:
Do you think we may add NVME storage type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "greenwich (via GitHub)" <gi...@apache.org>.
greenwich commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3192150435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
Review Comment:
Do you think we may introduce the NVME storage type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "greenwich (via GitHub)" <gi...@apache.org>.
greenwich commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3192150435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
Review Comment:
Does it make sense to introduce a new type NVME here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10191:
URL: https://github.com/apache/ozone/pull/10191#issuecomment-4407658996
Thanks @xichen01 for the patch and @greenwich @chungen0126 for the reviews.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3196493711
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
+ DISK("DISK", StorageType.DISK),
+ ARCHIVE("ARCHIVE", StorageType.ARCHIVE),
+ EMPTY("EMPTY");
+
+ private final String tierName;
+ private final List<StorageType> storageTypes;
+ private final boolean uniformStorageType;
+ private static final Map<StorageTier, Map<ReplicationConfig, List<StorageType>>>
+ CACHE = new EnumMap<>(StorageTier.class);
+
+ StorageTier(String tierName) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.emptyList();
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for uniform storage tiers
+ StorageTier(String tierName, StorageType uniformStorageType) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.singletonList(uniformStorageType);
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for non-uniform storage tiers
+ StorageTier(String tierName, StorageType... storageTypes) {
+ this.tierName = tierName;
+ if (Arrays.stream(storageTypes).distinct().count() <= 1) {
+ throw new IllegalArgumentException("StorageTier '" + tierName +
+ "' requires at least two different StorageType instances." +
+ " but only " + Arrays.stream(storageTypes).distinct().count() +
+ " StorageType were provided.");
+ }
+ this.storageTypes = Arrays.asList(storageTypes);
+ this.uniformStorageType = false;
+ }
+
+ static {
+ // Precompute storage type mappings for each replication config
+ for (StorageTier tier : StorageTier.values()) {
+ Map<ReplicationConfig, List<StorageType>> tierCache = new HashMap<>();
+ List<ReplicationConfig> replicationConfigs = Arrays.asList(
+ RatisReplicationConfig.getInstance(ONE),
+ RatisReplicationConfig.getInstance(THREE),
+ StandaloneReplicationConfig.getInstance(ONE),
+ StandaloneReplicationConfig.getInstance(THREE)
+ );
+
+ for (ReplicationConfig config : replicationConfigs) {
+ tierCache.put(config, tier.computeStorageTypes(config));
+ }
+ CACHE.put(tier, tierCache);
+ }
+ }
+
+ public StorageTierProto toProto() {
+ switch (this) {
+ case SSD:
+ return StorageTierProto.SSD_TIER;
+ case DISK:
+ return StorageTierProto.DISK_TIER;
+ case ARCHIVE:
+ return StorageTierProto.ARCHIVE_TIER;
+ default:
+ throw new IllegalStateException(
+ "Illegal StorageTier: " + this);
+ }
+ }
+
+ public static StorageTier fromProto(StorageTierProto tier) {
+ switch (tier) {
+ case SSD_TIER:
+ return SSD;
+ case DISK_TIER:
+ return DISK;
+ case ARCHIVE_TIER:
+ return ARCHIVE;
+ default:
+ throw new IllegalStateException(
+ "Illegal StorageTierProto: " + tier);
+ }
+ }
+
+ public String getTierName() {
+ return tierName;
+ }
+
+ public boolean isUniformStorageType() {
+ return uniformStorageType;
+ }
+
+ /**
+ * Computes the list of StorageTypes based on replication configuration.
+ *
+ * @param replicationConfig The replication configuration.
+ * @return The list of StorageTypes for the given tier and replication configuration.
+ */
+ private List<StorageType> computeStorageTypes(
+ ReplicationConfig replicationConfig) {
+ if (isUniformStorageType()) {
+ int numberOfNodes = replicationConfig.getRequiredNodes();
+ if (storageTypes.isEmpty()) {
+ return Collections.emptyList();
+ }
+ return new ArrayList<>(Collections.nCopies(numberOfNodes, storageTypes.get(0)));
Review Comment:
Updated
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/hdds/protocol/StoragePolicyTest.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.protocol;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.apache.hadoop.hdds.client.OzoneStoragePolicy;
+import org.apache.hadoop.hdds.client.StorageTier;
+import org.junit.jupiter.api.Test;
+
+class StoragePolicyTest {
Review Comment:
Added a test for this
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
Review Comment:
I think it depends on the actual needs. If you need to differentiate between regular low-speed SATA/SAS SSDs and high-speed NVMe SSDs, you can add an NVMe SSD.
You can create a subtask in https://issues.apache.org/jira/browse/HDDS-11233, and then we can submit a separate PR to add this `NVME` storage type.
FYI: Since the subsequent code is based on the current storage types, it is recommended to add this feature later; otherwise, it may introduce some conflicts.
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneStoragePolicy.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StoragePolicyProto;
+
+/**
+ * Enum defining different storage policies using StorageTier.
+ */
+public enum OzoneStoragePolicy implements StoragePolicy {
+
+ HOT("Hot", StorageTier.SSD, StorageTier.DISK),
+ WARM("Warm", StorageTier.DISK, StorageTier.EMPTY),
+ COLD("Cold", StorageTier.ARCHIVE, StorageTier.EMPTY);
+
+ private final String name;
+ private final StorageTier creationTier;
+ private final StorageTier creationFallbackTier;
+
+ OzoneStoragePolicy(String name, StorageTier creationTier,
+ StorageTier creationFallbackTier) {
+ this.name = name;
+ this.creationTier = creationTier;
+ this.creationFallbackTier = creationFallbackTier;
+ }
+
+ @Override
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public StorageTier getCreationTier() {
+ return creationTier;
+ }
+
+ @Override
+ public StorageTier getCreationFallbackTier() {
+ return creationFallbackTier;
+ }
+
+ /**
+ * Converts the current StoragePolicyType to its protobuf representation.
+ * @return the corresponding StoragePolicyProto.
+ */
+ public StoragePolicyProto toProto() {
+ switch (this) {
+ case HOT:
+ return StoragePolicyProto.HOT;
+ case WARM:
+ return StoragePolicyProto.WARM;
+ case COLD:
+ return StoragePolicyProto.COLD;
+ default:
+ throw new IllegalArgumentException(
+ "Error: StoragePolicyType not found, type=" + this);
Review Comment:
Updated
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
+ DISK("DISK", StorageType.DISK),
+ ARCHIVE("ARCHIVE", StorageType.ARCHIVE),
+ EMPTY("EMPTY");
+
+ private final String tierName;
+ private final List<StorageType> storageTypes;
+ private final boolean uniformStorageType;
+ private static final Map<StorageTier, Map<ReplicationConfig, List<StorageType>>>
+ CACHE = new EnumMap<>(StorageTier.class);
+
+ StorageTier(String tierName) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.emptyList();
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for uniform storage tiers
+ StorageTier(String tierName, StorageType uniformStorageType) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.singletonList(uniformStorageType);
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for non-uniform storage tiers
+ StorageTier(String tierName, StorageType... storageTypes) {
+ this.tierName = tierName;
+ if (Arrays.stream(storageTypes).distinct().count() <= 1) {
+ throw new IllegalArgumentException("StorageTier '" + tierName +
+ "' requires at least two different StorageType instances." +
+ " but only " + Arrays.stream(storageTypes).distinct().count() +
+ " StorageType were provided.");
+ }
Review Comment:
Yes, non-uniform storage tiers are not currently supported; this is merely a reserved interface.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "greenwich (via GitHub)" <gi...@apache.org>.
greenwich commented on PR #10191:
URL: https://github.com/apache/ozone/pull/10191#issuecomment-4384109576
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #10191:
URL: https://github.com/apache/ozone/pull/10191#discussion_r3189885224
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneStoragePolicy.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StoragePolicyProto;
+
+/**
+ * Enum defining different storage policies using StorageTier.
+ */
+public enum OzoneStoragePolicy implements StoragePolicy {
+
+ HOT("Hot", StorageTier.SSD, StorageTier.DISK),
+ WARM("Warm", StorageTier.DISK, StorageTier.EMPTY),
+ COLD("Cold", StorageTier.ARCHIVE, StorageTier.EMPTY);
+
+ private final String name;
+ private final StorageTier creationTier;
+ private final StorageTier creationFallbackTier;
+
+ OzoneStoragePolicy(String name, StorageTier creationTier,
+ StorageTier creationFallbackTier) {
+ this.name = name;
+ this.creationTier = creationTier;
+ this.creationFallbackTier = creationFallbackTier;
+ }
+
+ @Override
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public StorageTier getCreationTier() {
+ return creationTier;
+ }
+
+ @Override
+ public StorageTier getCreationFallbackTier() {
+ return creationFallbackTier;
+ }
+
+ /**
+ * Converts the current StoragePolicyType to its protobuf representation.
+ * @return the corresponding StoragePolicyProto.
+ */
+ public StoragePolicyProto toProto() {
+ switch (this) {
+ case HOT:
+ return StoragePolicyProto.HOT;
+ case WARM:
+ return StoragePolicyProto.WARM;
+ case COLD:
+ return StoragePolicyProto.COLD;
+ default:
+ throw new IllegalArgumentException(
+ "Error: StoragePolicyType not found, type=" + this);
Review Comment:
nit: The class name is `OzoneStoragePolicy`.
```suggestion
"Error: OzoneStoragePolicy not found, type=" + this);
```
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
+ DISK("DISK", StorageType.DISK),
+ ARCHIVE("ARCHIVE", StorageType.ARCHIVE),
+ EMPTY("EMPTY");
+
+ private final String tierName;
+ private final List<StorageType> storageTypes;
+ private final boolean uniformStorageType;
+ private static final Map<StorageTier, Map<ReplicationConfig, List<StorageType>>>
+ CACHE = new EnumMap<>(StorageTier.class);
+
+ StorageTier(String tierName) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.emptyList();
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for uniform storage tiers
+ StorageTier(String tierName, StorageType uniformStorageType) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.singletonList(uniformStorageType);
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for non-uniform storage tiers
+ StorageTier(String tierName, StorageType... storageTypes) {
+ this.tierName = tierName;
+ if (Arrays.stream(storageTypes).distinct().count() <= 1) {
+ throw new IllegalArgumentException("StorageTier '" + tierName +
+ "' requires at least two different StorageType instances." +
+ " but only " + Arrays.stream(storageTypes).distinct().count() +
+ " StorageType were provided.");
+ }
+ this.storageTypes = Arrays.asList(storageTypes);
+ this.uniformStorageType = false;
+ }
+
+ static {
+ // Precompute storage type mappings for each replication config
+ for (StorageTier tier : StorageTier.values()) {
+ Map<ReplicationConfig, List<StorageType>> tierCache = new HashMap<>();
+ List<ReplicationConfig> replicationConfigs = Arrays.asList(
+ RatisReplicationConfig.getInstance(ONE),
+ RatisReplicationConfig.getInstance(THREE),
+ StandaloneReplicationConfig.getInstance(ONE),
+ StandaloneReplicationConfig.getInstance(THREE)
+ );
+
+ for (ReplicationConfig config : replicationConfigs) {
+ tierCache.put(config, tier.computeStorageTypes(config));
+ }
+ CACHE.put(tier, tierCache);
+ }
+ }
+
+ public StorageTierProto toProto() {
+ switch (this) {
+ case SSD:
+ return StorageTierProto.SSD_TIER;
+ case DISK:
+ return StorageTierProto.DISK_TIER;
+ case ARCHIVE:
+ return StorageTierProto.ARCHIVE_TIER;
+ default:
+ throw new IllegalStateException(
+ "Illegal StorageTier: " + this);
+ }
+ }
+
+ public static StorageTier fromProto(StorageTierProto tier) {
+ switch (tier) {
+ case SSD_TIER:
+ return SSD;
+ case DISK_TIER:
+ return DISK;
+ case ARCHIVE_TIER:
+ return ARCHIVE;
+ default:
+ throw new IllegalStateException(
+ "Illegal StorageTierProto: " + tier);
+ }
+ }
+
+ public String getTierName() {
+ return tierName;
+ }
+
+ public boolean isUniformStorageType() {
+ return uniformStorageType;
+ }
+
+ /**
+ * Computes the list of StorageTypes based on replication configuration.
+ *
+ * @param replicationConfig The replication configuration.
+ * @return The list of StorageTypes for the given tier and replication configuration.
+ */
+ private List<StorageType> computeStorageTypes(
+ ReplicationConfig replicationConfig) {
+ if (isUniformStorageType()) {
+ int numberOfNodes = replicationConfig.getRequiredNodes();
+ if (storageTypes.isEmpty()) {
+ return Collections.emptyList();
+ }
+ return new ArrayList<>(Collections.nCopies(numberOfNodes, storageTypes.get(0)));
Review Comment:
Should we consider making the `StorageType` list read-only to ensure immutability?
```suggestion
return Collections.unmodifiableList(new ArrayList<>(Collections.nCopies(numberOfNodes, storageTypes.get(0)));
```
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/hdds/protocol/StoragePolicyTest.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.protocol;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.apache.hadoop.hdds.client.OzoneStoragePolicy;
+import org.apache.hadoop.hdds.client.StorageTier;
+import org.junit.jupiter.api.Test;
+
+class StoragePolicyTest {
Review Comment:
Could you please add some unit tests to cover the Protobuf/Java conversion?
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.client;
+
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE;
+import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.EnumMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.StorageTierProto;
+
+/**
+ * Ozone specific storage tiers.
+ */
+public enum StorageTier {
+ SSD("SSD", StorageType.SSD),
+ DISK("DISK", StorageType.DISK),
+ ARCHIVE("ARCHIVE", StorageType.ARCHIVE),
+ EMPTY("EMPTY");
+
+ private final String tierName;
+ private final List<StorageType> storageTypes;
+ private final boolean uniformStorageType;
+ private static final Map<StorageTier, Map<ReplicationConfig, List<StorageType>>>
+ CACHE = new EnumMap<>(StorageTier.class);
+
+ StorageTier(String tierName) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.emptyList();
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for uniform storage tiers
+ StorageTier(String tierName, StorageType uniformStorageType) {
+ this.tierName = tierName;
+ this.storageTypes = Collections.singletonList(uniformStorageType);
+ this.uniformStorageType = true;
+ }
+
+ // Constructor for non-uniform storage tiers
+ StorageTier(String tierName, StorageType... storageTypes) {
+ this.tierName = tierName;
+ if (Arrays.stream(storageTypes).distinct().count() <= 1) {
+ throw new IllegalArgumentException("StorageTier '" + tierName +
+ "' requires at least two different StorageType instances." +
+ " but only " + Arrays.stream(storageTypes).distinct().count() +
+ " StorageType were provided.");
+ }
Review Comment:
While I understand that non-uniform storage tiers aren't supported yet (so this line might not be reachable), `Arrays.stream(storageTypes).distinct().count()` is called twice here. We could assign it to a variable instead.
```suggestion
long distinctStorageTypes = Arrays.stream(storageTypes).distinct().count();
if (distinctStorageTypes <= 1) {
throw new IllegalArgumentException("StorageTier '" + tierName +
"' requires at least two different StorageType instances." +
" but only " + distinctStorageTypes +
" StorageType were provided.");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15175. Introduce StorageClass and storage policy [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 merged PR #10191:
URL: https://github.com/apache/ozone/pull/10191
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org