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/06/28 21:15:13 UTC
[GitHub] [ozone] swamirishi opened a new pull request, #3565: HDDS-6889: EC: put key command with EC replication can use Replicatio…
swamirishi opened a new pull request, #3565:
URL: https://github.com/apache/ozone/pull/3565
…nConfig validator
## What changes were proposed in this pull request?
Validate keys while putting a key
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6889
## How was this patch tested?
Unit test, Integration Test
--
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 diff in pull request #3565: HDDS-6889. EC: put key command with EC replication can use ReplicationConfig validator
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3565:
URL: https://github.com/apache/ozone/pull/3565#discussion_r919266398
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,6 +865,44 @@ private void verifyReplication(String volumeName, String bucketName,
}
}
+ @ParameterizedTest
+ @CsvSource({"rs-3-3-1024k,false", "xor-3-5-2048k,false",
+ "rs-3-2-1024k,true", "rs-6-3-1024k,true", "rs-10-4-1024k,true"})
+ public void testPutKeyWithReplicationConfig(String replicationValue,
+ boolean isValidReplicationConfig)
+ throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ String keyName = UUID.randomUUID().toString();
+ String value = UUID.randomUUID().toString();
+ ReplicationConfig replicationConfig =
+ new ECReplicationConfig(replicationValue);
+ if (isValidReplicationConfig) {
+ OzoneOutputStream out = bucket.createKey(keyName,
+ value.getBytes(UTF_8).length, replicationConfig, new HashMap<>());
+ out.write(value.getBytes(UTF_8));
+ out.close();
+ OzoneKey key = bucket.getKey(keyName);
+ Assert.assertEquals(keyName, key.getName());
+ OzoneInputStream is = bucket.readKey(keyName);
+ byte[] fileContent = new byte[value.getBytes(UTF_8).length];
+ is.read(fileContent);
+ Assert.assertEquals(value, new String(fileContent, UTF_8));
+ } else {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> bucket.createKey(keyName, "dummy".getBytes(UTF_8).length,
+ replicationConfig, new HashMap<>()));
+ }
Review Comment:
Nit: remove below unnecessary empty lines?
--
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 #3565: HDDS-6889. EC: put key command with EC replication can use ReplicationConfig validator
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3565:
URL: https://github.com/apache/ozone/pull/3565
--
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 diff in pull request #3565: HDDS-6889. EC: put key command with EC replication can use ReplicationConfig validator
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3565:
URL: https://github.com/apache/ozone/pull/3565#discussion_r910637395
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})
Review Comment:
You should pass valid values also? probably we can pass Pairs indicating valid config or not. Based valid flag, you can assert differently.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})
+ public void testPutKeyWithInvalidReplicationConfig(
+ String replicationValue) throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+
+ String value = "dummy";
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+ OzoneBucket bucket = volume.getBucket(bucketName);
+
+ String keyName = UUID.randomUUID().toString();
+ ReplicationConfig replicationConfig =
+ new ECReplicationConfig(replicationValue);
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> bucket.createKey(keyName,
+ value.getBytes(UTF_8).length, replicationConfig, new HashMap<>()));
+ }
+
@Test
public void testPutKey() throws IOException {
String volumeName = UUID.randomUUID().toString();
String bucketName = UUID.randomUUID().toString();
Instant testStartTime = Instant.now();
String value = "sample value";
Review Comment:
Removed the unnecessary empty line
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfigValidator.java:
##########
@@ -53,7 +54,8 @@ public ReplicationConfig validate(ReplicationConfig replicationConfig) {
if (!validationRegexp.matcher(
replicationConfig.configFormat()).matches()) {
String replication = replicationConfig.getReplication();
- if (replicationConfig instanceof ECReplicationConfig) {
Review Comment:
Type is enum. We should be able to use == ?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
}
}
+ @ParameterizedTest
+ @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})
+ public void testPutKeyWithInvalidReplicationConfig(
+ String replicationValue) throws IOException {
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+
+ String value = "dummy";
Review Comment:
Why do you need string 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