xichen01 commented on code in PR #10410:
URL: https://github.com/apache/ozone/pull/10410#discussion_r3382186021
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StorageTier.java:
##########
@@ -121,50 +117,51 @@ public String getTierName() {
return tierName;
}
- public boolean isUniformStorageType() {
- return uniformStorageType;
+ public boolean isUniform() {
+ return isUniform;
}
/**
* Computes the list of StorageTypes based on replication configuration.
*
- * @param replicationConfig The replication configuration.
+ * @param nodeCount The node count of the storageTier required.
+ * @param storageTier The required StorageTier.
* @return The list of StorageTypes for the given tier and replication
configuration.
*/
- private List<StorageType> computeStorageTypes(
- ReplicationConfig replicationConfig) {
- if (isUniformStorageType()) {
- int numberOfNodes = replicationConfig.getRequiredNodes();
+ private List<StorageType> computeStorageTypes(int nodeCount, StorageTier
storageTier) {
+ if (isUniform()) {
if (storageTypes.isEmpty()) {
return Collections.emptyList();
}
- return Collections.nCopies(numberOfNodes, storageTypes.get(0));
+ return Collections.nCopies(nodeCount, storageTypes.get(0));
} else {
throw new UnsupportedOperationException(
- "Unsupported not UniformStorage Storage Tier: " + replicationConfig);
+ "Unsupported not uniform StorageTier: " + storageTier);
}
}
/**
* Maps a StorageTier to its corresponding StorageType based on replication
type.
*
- * @param replicationConfig The replication configuration.
+ * @param nodeCount The node count of the storageTier required.
* @return The list of StorageTypes corresponding to the given tier and
replication configuration.
* @throws IllegalArgumentException if the replication configuration is not
supported.
*/
- public List<StorageType> getStorageTypes(
- ReplicationConfig replicationConfig) {
- Map<ReplicationConfig, List<StorageType>> tierCache = CACHE.get(this);
+ public List<StorageType> getStorageTypes(int nodeCount) {
+ Map<Integer, List<StorageType>> tierCache = CACHE.get(this);
if (tierCache != null) {
- List<StorageType> cachedStorageType = tierCache.get(replicationConfig);
+ List<StorageType> cachedStorageType = tierCache.get(nodeCount);
if (cachedStorageType != null) {
return cachedStorageType;
}
}
- throw new IllegalArgumentException("Unsupported ReplicationConfig: " +
- replicationConfig + " for StorageTier: " + getTierName());
+ throw new IllegalArgumentException("Unsupported node count: " +
+ nodeCount + " for StorageTier: " + getTierName());
}
+ public static void setDefault(StorageTier storageTier) {
+ defaultTier = storageTier;
+ }
Review Comment:
Yes, this will be used in the future, I removed it.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManagerUtil.java:
##########
@@ -71,22 +72,24 @@ private static String
formatDatanodeDetails(List<DatanodeDetails> dns) {
* policy will be called again. This will continue until the placement policy
* is able to select enough nodes or the number of nodes requested is reduced
* to zero when an exception will be thrown.
- * @param policy The placement policy to use to select nodes.
- * @param requiredNodes The number of nodes required
- * @param usedNodes Any nodes already used by the container
- * @param excludedNodes Any Excluded nodes which cannot be selected
+ *
+ * @param policy The placement policy to use to select nodes.
+ * @param requiredNodes The number of nodes required
+ * @param usedNodes Any nodes already used by the container
+ * @param excludedNodes Any Excluded nodes which cannot be selected
* @param defaultContainerSize The cluster default max container size
- * @param container The container to select new replicas for
+ * @param container The container to select new replicas for
+ * @param storageType
Review Comment:
Add
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineProvider.java:
##########
@@ -61,11 +64,12 @@ public PipelineStateManager getPipelineStateManager() {
return stateManager;
}
- protected abstract Pipeline create(REPLICATION_CONFIG replicationConfig)
- throws IOException;
+ protected abstract Pipeline create(REPLICATION_CONFIG replicationConfig,
+ StorageTier storageTier) throws IOException;
Review Comment:
This would cause the ReplicationConfig and StorageTier code to become
tightly coupled, and since there may be cases where only StorageTier is used,
they should still be kept separate.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
