Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-6294. EC: Make cluster-wide EC configuration take effect #3089

Merged
merged 9 commits into from Feb 17, 2022

Conversation

kaijchen
Copy link
Contributor

@kaijchen kaijchen commented Feb 15, 2022

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?

integration tests and acceptance tests.

@@ -38,7 +38,7 @@ Prepare For Tests
Test Bucket Creation
${result} = Execute ozone sh volume create /${prefix}vol1
Should not contain ${result} Failed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to pass ratis options here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bydefault, server should use ratis right

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bydefault, server should use ratis right

I have added Test Key Creation Default Bucket to verify this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

@kaijchen kaijchen Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@kaijchen kaijchen Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the patch. I have added few comments please check. Thanks

@kaijchen
Copy link
Contributor Author

Hi @umamaheswararao, thanks for reviewing. I have explained the changes above. Feel free to leave more comments if you have any questions or suggestions.

@kaijchen
Copy link
Contributor Author

Rebased to HDDS-3816-ec.

@kaijchen
Copy link
Contributor Author

Intergration tests and acceptance tests were added.

@umamaheswararao
Copy link
Contributor

Thanks @kaijchen for updating the patch. I have a clarification question above, please check. Once thats clarified, I am ok with this patch. Thanks

@kaijchen
Copy link
Contributor Author

kaijchen commented Feb 17, 2022

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.

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

Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@umamaheswararao umamaheswararao merged commit 4b9d387 into apache:HDDS-3816-ec Feb 17, 2022
@kaijchen kaijchen deleted the HDDS-6294 branch February 25, 2022 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants