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 2021/12/02 07:48:21 UTC
[GitHub] [ozone] umamaheswararao opened a new pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
umamaheswararao opened a new pull request #2880:
URL: https://github.com/apache/ozone/pull/2880
## What changes were proposed in this pull request?
Added the default replication setting flexibility via SetBucketPropertyRequest. I will raise separate issue to expose this option via CLI.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6020
## How was this patch tested?
Added tests.
--
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] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r761790774
##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -704,6 +704,21 @@ public void setBucketQuota(String volumeName, String bucketName,
}
+ @Override
+ public void setReplicationConfig(
+ String volumeName, String bucketName, ReplicationConfig replicationConfig)
+ throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ Preconditions.checkNotNull(replicationConfig);
+ OmBucketArgs.Builder builder = OmBucketArgs.newBuilder();
+ builder.setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setDefaultReplicationCOnfig(
Review comment:
There seems to be a typo in ".setDefaultReplicationCOnfig" where the "O" in config is in uppercase. Not sure if this is a pre-existing problem, as I don't see this method defined in this PR.
--
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] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r761795613
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3753,4 +3757,65 @@ public void testOverWriteKeyWithAndWithOutVersioning() throws Exception {
createRequiredForVersioningTest(volumeName, bucketName, keyName, true);
checkExceptedResultForVersioningTest(volumeName, bucketName, keyName, 2);
}
+
+ @Test
+ public void testSetECReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ Assert.assertEquals(bucket.getName(), bucket.getName());
Review comment:
`Assert.assertEquals(bucket.getName(), bucket.getName());`
I don't think this assert is needed as its just checking the same variable equals itself.
--
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] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r761791420
##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java
##########
@@ -199,15 +218,24 @@ public Builder setQuotaInNamespace(long quota) {
return this;
}
+ public Builder setDefaultReplicationCOnfig(
Review comment:
Ah, this is where the new method is defined I mentioned above. Typo in the method name COnfig -> 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] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r761797213
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3753,4 +3757,65 @@ public void testOverWriteKeyWithAndWithOutVersioning() throws Exception {
createRequiredForVersioningTest(volumeName, bucketName, keyName, true);
checkExceptedResultForVersioningTest(volumeName, bucketName, keyName, 2);
}
+
+ @Test
+ public void testSetECReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ Assert.assertEquals(bucket.getName(), bucket.getName());
+ ReplicationConfig currentReplicationConfig = bucket.getReplicationConfig();
+ Assert.assertEquals(
+ new StandaloneReplicationConfig(HddsProtos.ReplicationFactor.ONE),
+ currentReplicationConfig);
+ bucket.setReplicationConfig(
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024));
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024),
+ bucket.getReplicationConfig());
+
+ //Reset replication config back.
+ bucket.setReplicationConfig(currentReplicationConfig);
+ }
+
+ @Test
+ public void testSetRatisReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ Assert.assertEquals(bucket.getName(), bucket.getName());
Review comment:
Same comment as above, I don't think this assert for `bucket.getName(), bucket.getName()` is 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
[GitHub] [ozone] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r761796889
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3753,4 +3757,65 @@ public void testOverWriteKeyWithAndWithOutVersioning() throws Exception {
createRequiredForVersioningTest(volumeName, bucketName, keyName, true);
checkExceptedResultForVersioningTest(volumeName, bucketName, keyName, 2);
}
+
+ @Test
+ public void testSetECReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ Assert.assertEquals(bucket.getName(), bucket.getName());
+ ReplicationConfig currentReplicationConfig = bucket.getReplicationConfig();
+ Assert.assertEquals(
+ new StandaloneReplicationConfig(HddsProtos.ReplicationFactor.ONE),
+ currentReplicationConfig);
+ bucket.setReplicationConfig(
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024));
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024),
+ bucket.getReplicationConfig());
+
+ //Reset replication config back.
+ bucket.setReplicationConfig(currentReplicationConfig);
+ }
+
+ @Test
+ public void testSetRatisReplicationConfigOnBucket()
Review comment:
Is it worth just merging this test into the one above? Eg set the config to EC, check it, then set it to Ratis and check it, and then restore to default and check it?
--
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] sodonnel commented on a change in pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r763378519
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3753,4 +3757,47 @@ public void testOverWriteKeyWithAndWithOutVersioning() throws Exception {
createRequiredForVersioningTest(volumeName, bucketName, keyName, true);
checkExceptedResultForVersioningTest(volumeName, bucketName, keyName, 2);
}
+
+ @Test
+ public void testSetECReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ ReplicationConfig currentReplicationConfig = bucket.getReplicationConfig();
+ Assert.assertEquals(
+ new StandaloneReplicationConfig(HddsProtos.ReplicationFactor.ONE),
+ currentReplicationConfig);
+ ECReplicationConfig ecReplicationConfig =
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024);
+ bucket.setReplicationConfig(ecReplicationConfig);
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(ecReplicationConfig, bucket.getReplicationConfig());
+
+ RatisReplicationConfig retisReplicationConfig =
+ new RatisReplicationConfig(HddsProtos.ReplicationFactor.THREE);
+ bucket.setReplicationConfig(retisReplicationConfig);
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(retisReplicationConfig, bucket.getReplicationConfig());
Review comment:
One small typo here retis -> 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] sodonnel commented on pull request #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
sodonnel commented on pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#issuecomment-985385115
This change looks mostly good to me. Just a couple of minor comments to address.
--
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 #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #2880:
URL: https://github.com/apache/ozone/pull/2880#discussion_r764158680
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -3753,4 +3757,47 @@ public void testOverWriteKeyWithAndWithOutVersioning() throws Exception {
createRequiredForVersioningTest(volumeName, bucketName, keyName, true);
checkExceptedResultForVersioningTest(volumeName, bucketName, keyName, 2);
}
+
+ @Test
+ public void testSetECReplicationConfigOnBucket()
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getBucket(volume);
+ ReplicationConfig currentReplicationConfig = bucket.getReplicationConfig();
+ Assert.assertEquals(
+ new StandaloneReplicationConfig(HddsProtos.ReplicationFactor.ONE),
+ currentReplicationConfig);
+ ECReplicationConfig ecReplicationConfig =
+ new ECReplicationConfig(3, 2, ECReplicationConfig.EcCodec.RS, 1024);
+ bucket.setReplicationConfig(ecReplicationConfig);
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(ecReplicationConfig, bucket.getReplicationConfig());
+
+ RatisReplicationConfig retisReplicationConfig =
+ new RatisReplicationConfig(HddsProtos.ReplicationFactor.THREE);
+ bucket.setReplicationConfig(retisReplicationConfig);
+
+ // Get the bucket and check the updated config.
+ bucket = volume.getBucket(bucket.getName());
+
+ Assert.assertEquals(retisReplicationConfig, bucket.getReplicationConfig());
Review comment:
Thanks for catching. I have just fixed it.
--
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 #2880: HDDS-6020: EC: Provide set replicationConfig option to bucket
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged pull request #2880:
URL: https://github.com/apache/ozone/pull/2880
--
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