This is an automated email from the ASF dual-hosted git repository.
sodonnell pushed a commit to branch HDDS-3816-ec
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/HDDS-3816-ec by this push:
new 0a0e62e EC: Add Codec and Stripe Size to ECReplicationConfig (#2674)
0a0e62e is described below
commit 0a0e62e00294c017b7059857e69162dc971512b3
Author: Stephen O'Donnell <[email protected]>
AuthorDate: Tue Sep 28 08:32:12 2021 +0100
EC: Add Codec and Stripe Size to ECReplicationConfig (#2674)
---
.../hadoop/hdds/client/ECReplicationConfig.java | 91 ++++++++++++++++++++--
.../hdds/client/TestECReplicationConfig.java | 66 +++++++++++++---
.../interface-client/src/main/proto/hdds.proto | 2 +
.../client/MultiNodePipelineBlockAllocator.java | 3 +
.../ozone/client/SinglePipelineBlockAllocator.java | 10 ++-
.../hadoop/ozone/shell/TestOzoneShellHA.java | 2 +-
6 files changed, 150 insertions(+), 24 deletions(-)
diff --git
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
index 8315f1b..ca0b520 100644
---
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
+++
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java
@@ -20,45 +20,109 @@ package org.apache.hadoop.hdds.client;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import java.util.EnumSet;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
/**
* Replication configuration for EC replication.
*/
public class ECReplicationConfig implements ReplicationConfig {
-
- private static final Pattern STRING_FORMAT =
Pattern.compile("(\\d+)-(\\d+)");
-
+
+ /**
+ * Enum defining the allowed list of ECCodecs.
+ */
+ public enum EcCodec {
+ RS, XOR;
+
+ public static String allValuesAsString() {
+ return EnumSet.allOf(EcCodec.class)
+ .stream()
+ .map(Enum::toString)
+ .collect(Collectors.joining(","));
+ }
+ }
+
+ // Acceptable patterns are like:
+ // rs-3-2-1024k
+ // RS-3-2-2048
+ // XOR-10-4-4096K
+ private static final Pattern STRING_FORMAT
+ = Pattern.compile("([a-zA-Z]+)-(\\d+)-(\\d+)-(\\d+)((?:k|K))?");
+
private int data;
private int parity;
+ private int ecChunkSize = 1024 * 1024;
+
+ private EcCodec codec = EcCodec.RS;
+
public ECReplicationConfig(int data, int parity) {
this.data = data;
this.parity = parity;
}
+ public ECReplicationConfig(int data, int parity, EcCodec codec,
+ int ecChunkSize) {
+ this.data = data;
+ this.parity = parity;
+ this.codec = codec;
+ this.ecChunkSize = ecChunkSize;
+ }
+
+ /**
+ * Create an ECReplicationConfig object from a string representing the
+ * various parameters. Acceptable patterns are like:
+ * rs-3-2-1024k
+ * RS-3-2-2048
+ * XOR-10-4-4096K
+ * IllegalArgumentException will be thrown if the passed string does not
+ * match the defined pattern.
+ * @param string
+ */
public ECReplicationConfig(String string) {
final Matcher matcher = STRING_FORMAT.matcher(string);
if (!matcher.matches()) {
throw new IllegalArgumentException("EC replication config should be " +
- "defined in the form 3-2, 6-3 or 10-4");
+ "defined in the form rs-3-2-1024k, rs-6-3-1024; or rs-10-4-1024k");
+ }
+
+ try {
+ codec = EcCodec.valueOf(matcher.group(1).toUpperCase());
+ } catch (IllegalArgumentException e) {
+ throw new IllegalArgumentException("The codec " + matcher.group(1) +
+ " is invalid. It must be one of " + EcCodec.allValuesAsString() +
".",
+ e);
}
- data = Integer.parseInt(matcher.group(1));
- parity = Integer.parseInt(matcher.group(2));
+ data = Integer.parseInt(matcher.group(2));
+ parity = Integer.parseInt(matcher.group(3));
if (data <= 0 || parity <= 0) {
throw new IllegalArgumentException("Data and parity part in EC " +
"replication config supposed to be positive numbers");
}
+
+ int chunkSize = Integer.parseInt((matcher.group(4)));
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("The ecChunkSize (" + chunkSize +
+ ") be greater than zero");
+ }
+ if (matcher.group(5) != null) {
+ // The "k" modifier is present, so multiple by 1024
+ chunkSize = chunkSize * 1024;
+ }
+ ecChunkSize = chunkSize;
}
public ECReplicationConfig(
HddsProtos.ECReplicationConfig ecReplicationConfig) {
this.data = ecReplicationConfig.getData();
this.parity = ecReplicationConfig.getParity();
+ this.codec = EcCodec.valueOf(ecReplicationConfig.getCodec().toUpperCase());
+ this.ecChunkSize = ecReplicationConfig.getEcChunkSize();
}
@Override
@@ -75,6 +139,8 @@ public class ECReplicationConfig implements
ReplicationConfig {
return HddsProtos.ECReplicationConfig.newBuilder()
.setData(data)
.setParity(parity)
+ .setCodec(codec.toString())
+ .setEcChunkSize(ecChunkSize)
.build();
}
@@ -86,6 +152,14 @@ public class ECReplicationConfig implements
ReplicationConfig {
return parity;
}
+ public int getEcChunkSize() {
+ return ecChunkSize;
+ }
+
+ public EcCodec getCodec() {
+ return codec;
+ }
+
@Override
public boolean equals(Object o) {
if (this == o) {
@@ -95,12 +169,13 @@ public class ECReplicationConfig implements
ReplicationConfig {
return false;
}
ECReplicationConfig that = (ECReplicationConfig) o;
- return data == that.data && parity == that.parity;
+ return data == that.data && parity == that.parity
+ && codec == that.getCodec() && ecChunkSize == that.getEcChunkSize();
}
@Override
public int hashCode() {
- return Objects.hash(data, parity);
+ return Objects.hash(data, parity, codec, ecChunkSize);
}
}
diff --git
a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/client/TestECReplicationConfig.java
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/client/TestECReplicationConfig.java
index 18956c2..9440a6e 100644
---
a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/client/TestECReplicationConfig.java
+++
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/client/TestECReplicationConfig.java
@@ -17,33 +17,75 @@
*/
package org.apache.hadoop.hdds.client;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.junit.Assert;
import org.junit.Test;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.hadoop.hdds.client.ECReplicationConfig.EcCodec.RS;
+import static org.apache.hadoop.hdds.client.ECReplicationConfig.EcCodec.XOR;
+import static org.junit.Assert.fail;
+
/**
* Unit test for ECReplicationConfig.
*/
public class TestECReplicationConfig {
+ @Test
+ public void testSuccessfulStringParsing() {
+ Map<String, ECReplicationConfig> valid = new HashMap();
+ valid.put("rs-3-2-1024", new ECReplicationConfig(3, 2, RS, 1024));
+ valid.put("RS-3-2-1024", new ECReplicationConfig(3, 2, RS, 1024));
+ valid.put("rs-3-2-1024k", new ECReplicationConfig(3, 2, RS, 1024 * 1024));
+ valid.put("rs-3-2-1024K", new ECReplicationConfig(3, 2, RS, 1024 * 1024));
+ valid.put("xor-10-4-1", new ECReplicationConfig(10, 4, XOR, 1));
+ valid.put("XOR-6-3-12345", new ECReplicationConfig(6, 3, XOR, 12345));
+
+ for (Map.Entry<String, ECReplicationConfig> e : valid.entrySet()) {
+ ECReplicationConfig ec = new ECReplicationConfig(e.getKey());
+ Assert.assertEquals(e.getValue().getData(), ec.getData());
+ Assert.assertEquals(e.getValue().getParity(), ec.getParity());
+ Assert.assertEquals(e.getValue().getCodec(), ec.getCodec());
+ Assert.assertEquals(e.getValue().getEcChunkSize(), ec.getEcChunkSize());
+ }
+ }
@Test
- public void testStringParsing() {
- final ECReplicationConfig ec = new ECReplicationConfig("3-2");
- Assert.assertEquals(ec.getData(), 3);
- Assert.assertEquals(ec.getParity(), 2);
+ public void testUnsuccessfulStringParsing() {
+ String[] invalid = {
+ "3-2-1024",
+ "rss-3-2-1024",
+ "rs-3-0-1024",
+ "rs-3-2-0k",
+ "rs-3-2",
+ "x3-2"
+ };
+ for (String s : invalid) {
+ try {
+ new ECReplicationConfig(s);
+ fail(s + " should not parse correctly");
+ } catch (IllegalArgumentException e) {
+ // ignore, this expected
+ }
+ }
}
- @Test(expected = IllegalArgumentException.class)
- public void testStringParsingWithString() {
- new ECReplicationConfig("x3-2");
- }
+ @Test
+ public void testSerializeToProtoAndBack() {
+ ECReplicationConfig orig = new ECReplicationConfig(6, 3,
+ ECReplicationConfig.EcCodec.XOR, 1024);
+ HddsProtos.ECReplicationConfig proto = orig.toProto();
- @Test(expected = IllegalArgumentException.class)
- public void testStringParsingWithZero() {
- new ECReplicationConfig("3-0");
+ ECReplicationConfig recovered = new ECReplicationConfig(proto);
+ Assert.assertEquals(orig.getData(), recovered.getData());
+ Assert.assertEquals(orig.getParity(), recovered.getParity());
+ Assert.assertEquals(orig.getCodec(), recovered.getCodec());
+ Assert.assertEquals(orig.getEcChunkSize(), recovered.getEcChunkSize());
+ Assert.assertTrue(orig.equals(recovered));
}
-
}
\ No newline at end of file
diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto
b/hadoop-hdds/interface-client/src/main/proto/hdds.proto
index 595fdd6..8c99bb3 100644
--- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto
+++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto
@@ -273,6 +273,8 @@ enum ReplicationFactor {
message ECReplicationConfig {
required int32 data = 1;
required int32 parity = 2;
+ required string codec = 3;
+ required int32 ecChunkSize = 4;
}
message DefaultReplicationConfig {
diff --git
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
index 7ba8adc..e3f1d78 100644
---
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
+++
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/MultiNodePipelineBlockAllocator.java
@@ -55,6 +55,9 @@ public class MultiNodePipelineBlockAllocator implements
MockBlockAllocator {
HddsProtos.Port.newBuilder().setName("RATIS").setValue(1234 +
i)
.build()).build());
}
+ if (keyArgs.getType() == HddsProtos.ReplicationType.EC) {
+ builder.setEcReplicationConfig(keyArgs.getEcReplicationConfig());
+ }
pipeline = builder.build();
}
diff --git
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
index 71de5e8..46ac98b 100644
---
a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
+++
b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/SinglePipelineBlockAllocator.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.ozone.client;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockID;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ContainerBlockID;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DatanodeDetailsProto;
@@ -48,7 +49,7 @@ public class SinglePipelineBlockAllocator
KeyArgs keyArgs) {
if (pipeline == null) {
- pipeline = Pipeline.newBuilder()
+ Pipeline.Builder bldr = Pipeline.newBuilder()
.setFactor(keyArgs.getFactor())
.setType(keyArgs.getType())
.setId(PipelineID.newBuilder()
@@ -68,8 +69,11 @@ public class SinglePipelineBlockAllocator
.setName("RATIS")
.setValue(1234)
.build())
- .build())
- .build();
+ .build());
+ if (keyArgs.getType() == HddsProtos.ReplicationType.EC) {
+ bldr.setEcReplicationConfig(keyArgs.getEcReplicationConfig());
+ }
+ pipeline = bldr.build();
}
List<KeyLocation> results = new ArrayList<>();
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
index 8491ade..a8f6467 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
@@ -869,7 +869,7 @@ public class TestOzoneShellHA {
getVolume(volumeName);
String[] args =
new String[] {"bucket", "create", "/volume100/bucket0", "-rt", "EC",
- "-r", "3-2"};
+ "-r", "rs-3-2-1024k"};
execute(ozoneShell, args);
OzoneVolume volume =
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
