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/27 15:10:35 UTC

[GitHub] [ozone] kaijchen opened a new pull request, #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

kaijchen opened a new pull request, #3559:
URL: https://github.com/apache/ozone/pull/3559

   ## What changes were proposed in this pull request?
   
   We have introduced cluster-level replication for EC in HDDS-6294.
   When client side and bucket level replication config are both missing,
   client will pass `null` as replication config to let OM will decide it.
   
   Since streaming is not compatible for writing EC keys,
   we cannot determine when to use streaming just by key size and client side replication config.
   
   In this PR, I have added a explicit `--stream` flag in ozone shell key handler.
   In stream mode, replication config will fallback to client side defaults when missing.
   And it will be checked to ensure not using streaming to write an EC key.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6955
   
   ## How was this patch tested?
   
   Manual test + acceptance 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] captainzmc commented on pull request #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

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

   Thanks @kaijchen working on this.  Since HDDS-4454 has been rebase, can you rebase the 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] kaijchen commented on pull request #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

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

   @captainzmc @szetszwo PTAL, 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] szetszwo commented on a diff in pull request #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

Posted by GitBox <gi...@apache.org>.
szetszwo commented on code in PR #3559:
URL: https://github.com/apache/ozone/pull/3559#discussion_r911387688


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -96,26 +100,21 @@ protected void execute(OzoneClient client, OzoneAddress address)
     int chunkSize = (int) getConf().getStorageSize(OZONE_SCM_CHUNK_SIZE_KEY,
         OZONE_SCM_CHUNK_SIZE_DEFAULT, StorageUnit.BYTES);
 
-    Boolean useAsync = false;
-    if (dataFile.length() <= chunkSize ||
-        (replicationConfig != null &&
-        replicationConfig.getReplicationType()  == EC) ||
-        bucket.getReplicationConfig() instanceof ECReplicationConfig) {
-      useAsync = true;
-    }
-    if (useAsync) {
+    if (stream) {
       if (isVerbose()) {
-        out().println("API: async");
+        out().println("API: streaming");
       }
-      try (InputStream input = new FileInputStream(dataFile);
-           OutputStream output = bucket.createKey(keyName, dataFile.length(),
-               replicationConfig, keyMetadata)) {
-        IOUtils.copyBytes(input, output, chunkSize);
+      // In streaming mode, always resolve replication config at client side,
+      // because streaming is not compatible for writing EC keys.
+      if (replicationConfig == null) {
+        replicationConfig = bucket.getReplicationConfig();
       }
-    } else {
-      if (isVerbose()) {
-        out().println("API: streaming");
+      if (replicationConfig == null) {
+        replicationConfig = ReplicationConfig.parse(null, null, getConf());

Review Comment:
   Use ReplicationConfig.getDefault(..) and check bucket default.  Let's add a resolve method to ReplicationConfig:
   ```
     static ReplicationConfig resolve(ReplicationConfig replicationConfig,
         ReplicationConfig bucketReplicationConfig, ConfigurationSource conf) {
       if (replicationConfig == null) {
         replicationConfig = bucketReplicationConfig;
       }
       if (replicationConfig == null) {
         replicationConfig = getDefault(conf);
       }
       return replicationConfig;
     }
   ```



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -96,26 +100,21 @@ protected void execute(OzoneClient client, OzoneAddress address)
     int chunkSize = (int) getConf().getStorageSize(OZONE_SCM_CHUNK_SIZE_KEY,
         OZONE_SCM_CHUNK_SIZE_DEFAULT, StorageUnit.BYTES);
 
-    Boolean useAsync = false;
-    if (dataFile.length() <= chunkSize ||
-        (replicationConfig != null &&
-        replicationConfig.getReplicationType()  == EC) ||
-        bucket.getReplicationConfig() instanceof ECReplicationConfig) {
-      useAsync = true;
-    }
-    if (useAsync) {
+    if (stream) {

Review Comment:
   Let's move the stream code to a separated method.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -130,6 +129,15 @@ protected void execute(OzoneClient client, OzoneAddress address)
           len -= writeLen;
         }
       }
+    } else {
+      if (isVerbose()) {
+        out().println("API: async");
+      }
+      try (InputStream input = new FileInputStream(dataFile);
+           OutputStream output = bucket.createKey(keyName, dataFile.length(),
+               replicationConfig, keyMetadata)) {
+        IOUtils.copyBytes(input, output, chunkSize);
+      }

Review Comment:
   Move it to a separated method.



-- 
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 #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

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

   @szetszwo Thanks for the improvement.


-- 
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 #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

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

   > Thanks @kaijchen working on this. Since [HDDS-4454](https://issues.apache.org/jira/browse/HDDS-4454) has been rebase, can you rebase the PR?
   
   Sure. I have rebased this PR to HDDS-4454.


-- 
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 merged pull request #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

Posted by GitBox <gi...@apache.org>.
captainzmc merged PR #3559:
URL: https://github.com/apache/ozone/pull/3559


-- 
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 #3559: HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell

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

   Thanks @szetszwo and @captainzmc for the review.


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