You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/02/15 08:48:20 UTC
[GitHub] [ozone] kaijchen opened a new pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
kaijchen opened a new pull request #3089:
URL: https://github.com/apache/ozone/pull/3089
## What changes were proposed in this pull request?
Use `ozone.server.default.replication` and `ozone.server.default.replication.type` as cluster level replication configs.
ReplicationConfig will be decided in the following order:
1. Client-passed config
2. Bucket-level config
3. Cluster-level config
When creating new buckets, the default bucket-level config is set to `null`.
When listing buckets, buckets with no bucket-level replicationConfig will not show `replicationConfig`.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6294
## How was this patch tested?
manual tests, maybe UT should be added.
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807548512
##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java
##########
@@ -121,7 +121,7 @@ public void execute(OzoneClient client, OzoneAddress address)
}
}
- if (replicationType != null) {
+ if(replicationType!=null) {
Review comment:
Yes, I guess they got fixed as part of master merge to EC branch. Please rebase.
--
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
[GitHub] [ozone] kaijchen removed a comment on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen removed a comment on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1041128177
I have rebased it to the latest HDDS-3816-ec branch.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r808614039
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
If `--replication 3 --type RATIS` is missing here, the bucket-level replicationConfig will be `null`. When creating a key in this bucket, cluster-level config will be used, it will still create RATIS key because cluster-level config defaults to RATIS.
If we change the cluster-level config by setting `ozone.server.default.replication = rs-3-2-1024k` and `ozone.server.default.replication.type = EC`. After OM restart, EC keys will be created by default in buckets without replication config.
There will be 3 types of bucket after this change,
| bucket type | bucket replicationConfig | cluster replicationConfig |
| --- | --- | --- |
| default | null | used |
| ratis | ratis | ignored |
| ec | ec | ignored |
--
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
[GitHub] [ozone] kaijchen commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1041125377
Hi @umamaheswararao, thanks for reviewing. I have explained the changes above. Feel free to leave more comments if you have any questions or suggestions.
--
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
[GitHub] [ozone] kaijchen commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1041240213
Intergration tests and acceptance tests were added.
--
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
[GitHub] [ozone] kaijchen commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1042529126
Hi @umamaheswararao, here is the steps you can verify the change.
For default buckets, it will now use cluster-level config at the key creation time.
For buckets with explicit replication config, it's the same as before.
```bash
ozone sh volume create vol1
ozone sh bucket create vol1/bucket1
ozone sh key put vol1/bucket1/key1 data # ratis key
# set ozone.server.default.replication = rs-3-2-1024k
# set ozone.server.default.replication.type = EC
# restart OM
ozone sh key put vol1/bucket1/key2 data # EC key
```
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r808177442
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
@kaijchen, Thanks for proving the details. I am not sure I fully get it here, but here is my another question to get it clarified:
What if you don't pass ratis option here in the below line?
${result} = Execute ozone sh bucket create --replication 3 --type RATIS /${prefix}vol1/${prefix}ratis
Does this still create ratis key? I understand you already added a test with default. If we remove the options above, this should also be like default. But you are just testing these options as well. Is this correct? ( I am not very good in understanding these script based tests yet. )
I am trying to make sure the behavior is same as expected.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807549135
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -120,19 +120,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
final long requestedSize = keyArgs.getDataSize() > 0 ?
keyArgs.getDataSize() : scmBlockSize;
- boolean useRatis = ozoneManager.shouldUseRatis();
Review comment:
keyArgs.getFactor() and keyArgs.getType() have `@NotNull` annotation on return value.
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807543884
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -44,7 +44,13 @@
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.bucket.OMBucketCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*;
Review comment:
I see this exist in master. Could you please avoid this change in this JIRA? Let's do this in master when we get chance, rather than doing here in this branch unrelated way. ( It could create unnecessary conflicts some times while merging)
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807544059
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Correct. I have changed bucket default replication config to `null` instead of `RATIS`.
If bucket-level replication config is missing, cluster-level config will be used.
If bucket-level replication config is `null`, it will not show in the result of listing buckets.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r808619356
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Previously, the bucket-level replication config is set at the bucket creation. If it's missing, client-side default will be used as bucket-level replication config.
Now, the bucket-level replication config can be empty. If it's missing, bucket-level replication config will just be `null`. The cluster-level replication config will be used when creating keys in that bucket.
Note there is a compatibility change in `ozone sh bucket list` and `ozone sh bucket info`. Those buckets without bucket-level replication config will not show `replicationConfig` in the result.
Here is the matrix of default key replication config in different buckets.
| cluster level config | default bucket | EC bucket | RATIS bucket |
| --- | --- | --- | --- |
| RATIS (default) | RATIS | EC | RATIS |
| EC | EC | EC | RATIS |
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r808675799
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Thanks for the details. Yes, this is expected behavior. I am fine with this. Compat issue you are pointing is within the ec branch, so no issues with that.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807547989
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
> Why do we need to pass ratis options here?
Because if no bucket-level replication config is set, list bucket will not show replicationConfig anymore. Since we are checking the result of list bucket, we need to pass ratis option 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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807550856
##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java
##########
@@ -121,7 +121,7 @@ public void execute(OzoneClient client, OzoneAddress address)
}
}
- if (replicationType != null) {
+ if(replicationType!=null) {
Review comment:
I have reverted this change.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807560555
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3677,7 +3682,16 @@ public boolean getEnableFileSystemPaths() {
}
public ReplicationConfig getDefaultReplicationConfig() {
- return ReplicationConfig.getDefault(configuration);
+ final String replication = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_DEFAULT
+ );
+ final String type = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_DEFAULT
+ );
Review comment:
configuration.get only takes 2 arguments.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807659260
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
> Bydefault, server should use ratis right
I have added `Test Key Creation Default Bucket` to verify this.
--
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
[GitHub] [ozone] umamaheswararao commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1042499168
Thanks @kaijchen for updating the patch. I have a clarification question above, please check. Once thats clarified, I am ok with this patch. Thanks
--
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
[GitHub] [ozone] umamaheswararao merged pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged pull request #3089:
URL: https://github.com/apache/ozone/pull/3089
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807542107
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Bydefault, server should use ratis right
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807541805
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Why do we need to pass ratis options 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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807544825
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -120,19 +120,8 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
final long requestedSize = keyArgs.getDataSize() > 0 ?
keyArgs.getDataSize() : scmBlockSize;
- boolean useRatis = ozoneManager.shouldUseRatis();
Review comment:
I agree to remove these unnecessary checks. They should not be null.
--
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
[GitHub] [ozone] kaijchen commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1041128177
I have rebased it to the latest HDDS-3816-ec branch.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807544276
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -44,7 +44,13 @@
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.bucket.OMBucketCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*;
Review comment:
Sure.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807716190
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -44,7 +44,13 @@
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.bucket.OMBucketCreateResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*;
Review comment:
I have fired a JIRA HDDS-6329 to address all the star imports.
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807548512
##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java
##########
@@ -121,7 +121,7 @@ public void execute(OzoneClient client, OzoneAddress address)
}
}
- if (replicationType != null) {
+ if(replicationType!=null) {
Review comment:
Yes, I guess they got fixed as part of master merge to EC branch. Please rebase with ec branch.
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807562506
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3677,7 +3682,16 @@ public boolean getEnableFileSystemPaths() {
}
public ReplicationConfig getDefaultReplicationConfig() {
- return ReplicationConfig.getDefault(configuration);
+ final String replication = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_DEFAULT
+ );
+ final String type = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_DEFAULT
+ );
Review comment:
We don't need, since the `replication` and `type` can't be `null`.
And it's also weird to fallback to client side config 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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r808619356
##########
File path: hadoop-ozone/dist/src/main/smoketest/ec/basic.robot
##########
@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Review comment:
Previously, the bucket-level replication config is set at the bucket creation. If config is missing, client-side default will be used.
Now, the bucket-level replication config can be empty. If config is missing, bucket-level replication config will just be `null`. The cluster-level replication config will be used when creating keys in that bucket.
Note there is a compatibility change in `ozone sh bucket list` and `ozone sh bucket info`. Those buckets without bucket-level replication config will not show `replicationConfig` in the result.
Here is the matrix of default key replication config in different buckets.
| cluster level config | default bucket | EC bucket | RATIS bucket |
| --- | --- | --- | --- |
| RATIS (default) | RATIS | EC | RATIS |
| EC | EC | EC | RATIS |
--
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
[GitHub] [ozone] kaijchen edited a comment on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen edited a comment on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1042529126
Hi @umamaheswararao, here is the steps you can verify the change.
For default buckets, it will now use cluster-level config at the key creation time.
For buckets with explicit replication config, it's the same as before.
```bash
ozone sh volume create vol1
ozone sh bucket create vol1/bucket1
ozone sh bucket info vol1/bucket1 # no replicationConfig
ozone sh key put vol1/bucket1/key1 data # ratis key
# set ozone.server.default.replication = rs-3-2-1024k
# set ozone.server.default.replication.type = EC
# restart OM
ozone sh key put vol1/bucket1/key2 data # EC key
```
--
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
[GitHub] [ozone] umamaheswararao commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807546781
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3677,7 +3682,16 @@ public boolean getEnableFileSystemPaths() {
}
public ReplicationConfig getDefaultReplicationConfig() {
- return ReplicationConfig.getDefault(configuration);
+ final String replication = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_DEFAULT
+ );
+ final String type = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_DEFAULT
+ );
Review comment:
Shouldn't we pass the same configuration below 3rd arg?
--
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
[GitHub] [ozone] kaijchen commented on a change in pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#discussion_r807564451
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -3677,7 +3682,16 @@ public boolean getEnableFileSystemPaths() {
}
public ReplicationConfig getDefaultReplicationConfig() {
- return ReplicationConfig.getDefault(configuration);
+ final String replication = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_DEFAULT
+ );
+ final String type = configuration.get(
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_KEY,
+ OZONE_SERVER_DEFAULT_REPLICATION_TYPE_DEFAULT
+ );
Review comment:
Oops, we still need to use the validator in the config.
--
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
[GitHub] [ozone] kaijchen commented on pull request #3089: HDDS-6294. EC: Make cluster-wide EC configuration take effect
Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #3089:
URL: https://github.com/apache/ozone/pull/3089#issuecomment-1041238662
Rebased to HDDS-3816-ec.
--
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