sodonnel commented on a change in pull request #3008:
URL: https://github.com/apache/ozone/pull/3008#discussion_r791736925
##########
File path:
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java
##########
@@ -64,4 +73,78 @@ public static BucketLayout
resolveLinkBucketLayout(OzoneBucket bucket,
}
return bucket.getBucketLayout();
}
+
+ /**
+ * This API used to resolve the client side configuration preference for file
+ * system layer implementations.
+ *
+ * @param replication - replication value passed from FS API.
+ * @param clientConfiguredReplConfig - Client side configured replication
+ * config.
+ * @param bucketReplConfig - server side bucket default replication
+ * config.
+ * @param config - Ozone configuration object.
+ * @return client resolved replication config.
+ */
+ public static ReplicationConfig resolveClientSideReplicationConfig(
Review comment:
Should we add a unit test for this logic? There is a fair bit going on
here to not have a test covering it.
##########
File path:
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java
##########
@@ -211,29 +239,20 @@ public OzoneFSOutputStream createFile(String key, short
replication,
boolean overWrite, boolean recursive) throws IOException {
incrementCounter(Statistic.OBJECTS_CREATED, 1);
try {
- OzoneOutputStream ozoneOutputStream = null;
- ReplicationConfig replConfig = this.replicationConfig;
- // Since the bucket has the right default replication, we are using it.
- if (bucket.getReplicationConfig() != null) {
- replConfig = bucket.getReplicationConfig();
- }
- if (replication == ReplicationFactor.ONE.getValue()
- || replication == ReplicationFactor.THREE.getValue()) {
-
- ReplicationConfig customReplicationConfig =
- ReplicationConfig.adjustReplication(
- replConfig, replication, config);
- ozoneOutputStream =
- bucket.createFile(key, 0, customReplicationConfig, overWrite,
- recursive);
- } else {
- ozoneOutputStream =
- bucket.createFile(key, 0, replConfig, overWrite, recursive);
- }
+ ReplicationConfig clientConfiguredReplConfig =
Review comment:
Nit: I don't think we need these two local variiables - we can use the
instance variable directly without copying it, and place the
`getReplicationConfigWithRefreshCheck()` inline with the method.
--
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]
