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/01/18 08:30:14 UTC

[GitHub] [ozone] captainzmc opened a new pull request #2996: HDDS-6194. EC: Freon ockg support EC write

captainzmc opened a new pull request #2996:
URL: https://github.com/apache/ozone/pull/2996


   ## What changes were proposed in this pull request?
   see: https://issues.apache.org/jira/browse/HDDS-6194
   
   ## How was this patch tested?
   NA. 
   


-- 
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] adoroszlai commented on a change in pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r786967496



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -66,23 +66,31 @@
       defaultValue = "4096")
   private int bufferSize;
 
-  @Option(names = { "-F", "--factor" },
-      description = "Replication factor (ONE, THREE)",
-      defaultValue = "THREE"
-  )
-  private ReplicationFactor factor = ReplicationFactor.THREE;
-

Review comment:
       It would be nice to keep the old option as `@Deprecated` for backward compatibility.  It should imply `--type RATIS`, and be mutually exclusive with `--replication`.




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r804322064



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -94,6 +113,15 @@ public Void call() throws Exception {
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
 
+    if (spec.commandLine().getParseResult().hasMatchedOption("--factor")) {
+      replicationConfig = ReplicationConfig
+          .fromTypeAndFactor(ReplicationType.RATIS, factor);
+    } else {
+      replicationConfig = OzoneClientUtils
+          .validateAndGetClientReplicationConfig(replicationType, replication,
+              ozoneConfiguration);
+    }

Review comment:
       > Something is wrong around option handling. Setting only `replication`, but neither `factor` nor `type`, a regular RATIS/3 key was created:
   
   This is due to `OzoneClientUtils#validateAndGetClientReplicationConfig`, the default client config was used.
   
   https://github.com/apache/ozone/blob/c4aec7bb976867623c0f13c4795963b97855a109/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneClientUtils.java#L172-L175




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1035953537


   Yes, that should be fine.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen edited a comment on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1034695133


   Hi @adoroszlai @sodonnel @umamaheswararao, since @captainzmc is on vacation, I have updated the code according to your comments above. Please take another look, 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] kaijchen commented on pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1034695133


   Hi @adoroszlai and @umamaheswararao, since @captainzmc is on vacation, I have updated the code according to your comments above. Please take another look, 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] kaijchen commented on a change in pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r804395635



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -112,7 +140,7 @@ private void createKey(long counter) throws Exception {
 
     timer.time(() -> {
       try (OutputStream stream = bucket.createKey(key, keySize,
-              ReplicationType.RATIS, factor, metadata)) {
+          replicationConfig, metadata)) {
         contentGenerator.write(stream);
         stream.flush();

Review comment:
       > Probably for now if Type is EC, ignore to invoke flush? [ We thought not to implement flush unless there is a critical req for it. ]
   
   Maybe just catch this exception? Since it's already in a try clause.
   And if flush is implemented in the future, it will work without any 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] umamaheswararao commented on pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1024528093


   HI @captainzmc, JFYI The pr #2990  is committed now.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r803660305



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -112,7 +140,7 @@ private void createKey(long counter) throws Exception {
 
     timer.time(() -> {
       try (OutputStream stream = bucket.createKey(key, keySize,
-              ReplicationType.RATIS, factor, metadata)) {
+          replicationConfig, metadata)) {
         contentGenerator.write(stream);
         stream.flush();

Review comment:
       Yes, this is expected. It should be fixed in another PR.
   
   https://github.com/apache/ozone/blob/22e34820cf228b23cc648c5833b63c8ed3523481/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java#L466-L469




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1036328952


   @adoroszlai do you any more comments? Otherwise I will just go ahead to merge.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r804391223



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -112,7 +140,7 @@ private void createKey(long counter) throws Exception {
 
     timer.time(() -> {
       try (OutputStream stream = bucket.createKey(key, keySize,
-              ReplicationType.RATIS, factor, metadata)) {
+          replicationConfig, metadata)) {
         contentGenerator.write(stream);
         stream.flush();

Review comment:
       Probably for now if Type is EC, ignore to invoke flush? [ We thought not to implement flush unless there is a critical req for 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] kaijchen commented on a change in pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r803669098



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -94,6 +113,15 @@ public Void call() throws Exception {
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
 
+    if (spec.commandLine().getParseResult().hasMatchedOption("--factor")) {
+      replicationConfig = ReplicationConfig
+          .fromTypeAndFactor(ReplicationType.RATIS, factor);
+    } else {
+      replicationConfig = OzoneClientUtils
+          .validateAndGetClientReplicationConfig(replicationType, replication,
+              ozoneConfiguration);
+    }

Review comment:
       It will be meaningless to set `replication` or `type` only. 
   And it is possible to set `--type EC` with `--replication THREE` together.
   
   | type/replication | THREE | rs-3-2-1024k |
   | --- | --- | --- |
   | RATIS | OK | Bad |
   | EC | Bad | OK |
   
   Should we bind this two options together like `RATIS/THREE` or `EC/rs-3-2-1024k` instead?




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r803669098



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -94,6 +113,15 @@ public Void call() throws Exception {
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
 
+    if (spec.commandLine().getParseResult().hasMatchedOption("--factor")) {
+      replicationConfig = ReplicationConfig
+          .fromTypeAndFactor(ReplicationType.RATIS, factor);
+    } else {
+      replicationConfig = OzoneClientUtils
+          .validateAndGetClientReplicationConfig(replicationType, replication,
+              ozoneConfiguration);
+    }

Review comment:
       It is meaningless to set `replication` or `type` only. 
   And it's also possible to set `--type EC` with `--replication THREE` together.
   
   | type/replication | THREE | rs-3-2-1024k |
   | --- | --- | --- |
   | RATIS | OK | Bad |
   | EC | Bad | OK |
   
   Should we bind this two options together like `RATIS/THREE` or `EC/rs-3-2-1024k` instead?




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r803669098



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -94,6 +113,15 @@ public Void call() throws Exception {
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
 
+    if (spec.commandLine().getParseResult().hasMatchedOption("--factor")) {
+      replicationConfig = ReplicationConfig
+          .fromTypeAndFactor(ReplicationType.RATIS, factor);
+    } else {
+      replicationConfig = OzoneClientUtils
+          .validateAndGetClientReplicationConfig(replicationType, replication,
+              ozoneConfiguration);
+    }

Review comment:
       It will be meaningless to only setting `replication` or `type` only. 
   And it is possible to set `--type EC` with `--replication THREE` together.
   
   | type/replication | THREE | rs-3-2-1024k |
   | --- | --- | --- |
   | RATIS | OK | Bad |
   | EC | Bad | OK |
   
   Should we bind this two options together like `RATIS/THREE` or `EC/rs-3-2-1024k` instead?




-- 
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] adoroszlai commented on a change in pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r803543129



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -112,7 +140,7 @@ private void createKey(long counter) throws Exception {
 
     timer.time(() -> {
       try (OutputStream stream = bucket.createKey(key, keySize,
-              ReplicationType.RATIS, factor, metadata)) {
+          replicationConfig, metadata)) {
         contentGenerator.write(stream);
         stream.flush();

Review comment:
       Got this for EC key:
   
   ```
   $ ozone freon ockg -n1 -t1 --replication 'rs-3-2-1024k' --type EC
   ...
   NotImplementedException: The flush API is not implemented yet.
   	at org.apache.hadoop.ozone.client.io.ECKeyOutputStream.flush(ECKeyOutputStream.java:468)
   	at org.apache.hadoop.ozone.client.io.OzoneOutputStream.flush(OzoneOutputStream.java:55)
   	at org.apache.hadoop.ozone.freon.OzoneClientKeyGenerator.lambda$createKey$0(OzoneClientKeyGenerator.java:145)
   	at com.codahale.metrics.Timer.time(Timer.java:101)
   	at org.apache.hadoop.ozone.freon.OzoneClientKeyGenerator.createKey(OzoneClientKeyGenerator.java:141)
   ```

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -94,6 +113,15 @@ public Void call() throws Exception {
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
 
+    if (spec.commandLine().getParseResult().hasMatchedOption("--factor")) {
+      replicationConfig = ReplicationConfig
+          .fromTypeAndFactor(ReplicationType.RATIS, factor);
+    } else {
+      replicationConfig = OzoneClientUtils
+          .validateAndGetClientReplicationConfig(replicationType, replication,
+              ozoneConfiguration);
+    }

Review comment:
       Something is wrong around option handling.  Setting only `replication`, but neither `factor` nor `type`, a regular RATIS/3 key was created:
   
   ```
   $ ozone freon ockg -n1 -t1 --replication 'rs-3-2-1024k' -p replication_rs-3-2-1024k
   ...
   Successful executions: 1
   
   $ ozone sh key info /vol1/bucket1/replication_rs-3-2-1024k/0
   ...
     "replicationConfig" : {
       "replicationFactor" : "THREE",
       "requiredNodes" : 3,
       "replicationType" : "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 edited a comment on pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao edited a comment on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1024528093


   HI @captainzmc, JFYI The pr #2990  is committed now. You may want to resume your work. 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] kaijchen commented on pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1035939822


   > Why can't we do something like this?
   > 
   > ```
   > if (type != EC){ 
   > // Add a comment saying, flush is not implemented in EC
   >  stream.flush();
   >  }
   > ```
   > 
   > Handling NotImplementedException seems not correct to me. What if something else throws that exception? How would differentiate it's coming from EC only?
   
   We cannot determine it is not a EC key by just checking `type`.
   For example, writing to a EC bucket with no client side `type` configuration.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1036954494


   Thanks @umamaheswararao and @adoroszlai for reviewing.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r804344506



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -112,7 +140,7 @@ private void createKey(long counter) throws Exception {
 
     timer.time(() -> {
       try (OutputStream stream = bucket.createKey(key, keySize,
-              ReplicationType.RATIS, factor, metadata)) {
+          replicationConfig, metadata)) {
         contentGenerator.write(stream);
         stream.flush();

Review comment:
       Should we catch this exception here in ockg?




-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao merged pull request #2996:
URL: https://github.com/apache/ozone/pull/2996


   


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
kaijchen removed a comment on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1035939822


   > Why can't we do something like this?
   > 
   > ```
   > if (type != EC){ 
   > // Add a comment saying, flush is not implemented in EC
   >  stream.flush();
   >  }
   > ```
   > 
   > Handling NotImplementedException seems not correct to me. What if something else throws that exception? How would differentiate it's coming from EC only?
   
   We cannot determine it is not a EC key by just checking `type`.
   For example, writing to a EC bucket with no client side `type` configuration.


-- 
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 #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on a change in pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#discussion_r787015365



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -66,23 +66,31 @@
       defaultValue = "4096")
   private int bufferSize;
 
-  @Option(names = { "-F", "--factor" },
-      description = "Replication factor (ONE, THREE)",
-      defaultValue = "THREE"
-  )
-  private ReplicationFactor factor = ReplicationFactor.THREE;
-
   @Option(
       names = "--om-service-id",
       description = "OM Service ID"
   )
   private String omServiceID = null;
 
+  @Option(names = {"--replication"},
+      description =
+          "Replication configuration of the new key. (this is replication "
+              + "specific. for RATIS/STANDALONE you can use ONE or THREE, " +

Review comment:
       STANDALONE --> STAND_ALONE ?

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/OzoneClientKeyGenerator.java
##########
@@ -93,6 +101,8 @@ public Void call() throws Exception {
 
     contentGenerator = new ContentGenerator(keySize, bufferSize);
     metadata = new HashMap<>();
+    replicationConfig = ReplicationConfig.parse(replicationType, replication,

Review comment:
       parse is returning default values even if we pass replication and type as null.
   When we set the EC on bucket, ideally it's not mandatory to pass the type and replication with key as we moved the default configs to server side along with bucket.
   
   Please refer the JIRA: HDDS-6184 




-- 
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] captainzmc commented on pull request #2996: HDDS-6194. EC: Freon ockg support EC write

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2996:
URL: https://github.com/apache/ozone/pull/2996#issuecomment-1016020831


   Thanks @sodonnel @adoroszlai @umamaheswararao for the review, while #2990 merged I will update 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