[GitHub] [ozone] szetszwo commented on a change in pull request #2858: HDDS-5869. [Ozone-Streaming] Added support for stream on S3Gateway wr…

[GitBox <gi...@apache.org>]

szetszwo commented on a change in pull request #2858:
URL: https://github.com/apache/ozone/pull/2858#discussion_r754219961



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -140,6 +145,10 @@ public void init() {
     bufferSize = (int) ozoneConfiguration.getStorageSize(
         OZONE_S3G_CLIENT_BUFFER_SIZE_KEY,
         OZONE_S3G_CLIENT_BUFFER_SIZE_DEFAULT, StorageUnit.BYTES);
+    chunkSize = (int) ozoneConfiguration.getStorageSize(
+        OZONE_SCM_CHUNK_SIZE_KEY,
+        OZONE_SCM_CHUNK_SIZE_DEFAULT,
+        org.apache.hadoop.conf.StorageUnit.BYTES);

Review comment:
       Typo?  Why hadoop.conf but not hadoop.hdds.conf?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -157,8 +166,6 @@ public Response put(
       @QueryParam("uploadId") @DefaultValue("") String uploadID,

Review comment:
       We should make streaming optional.  Add a new QueryParam with DefaultValue not using streaming.
   ```
   +      @QueryParam("streaming") @DefaultValue("false") boolean streaming,
   ```
   Use streaming only if it is true.

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -223,13 +233,56 @@ public Response put(
         }
       }
       throw ex;
-    } finally {
-      if (output != null) {
-        output.close();
-      }
     }
   }
 
+
+  private static void putKeyWithStream(OzoneBucket bucket,
+                                String keyPath,
+                                long length,
+                                int bufferSize,
+                                ReplicationConfig replicationConfig,
+                                Map<String, String> keyMetadata,
+                                InputStream body)
+      throws IOException {
+    try (OzoneDataStreamOutput streamOutput = bucket.createStreamKey(keyPath,
+            length, replicationConfig, keyMetadata)) {
+      writeToStreamOutput(streamOutput, body, bufferSize, length);
+    }
+  }
+
+  private static void writeToStreamOutput(OzoneDataStreamOutput streamOutput,
+                                          InputStream body,
+                                          int bufferSize)
+          throws IOException {
+    // (length == -1) Read up to EOF
+    writeToStreamOutput(streamOutput, body, bufferSize, -1);
+  }
+
+  private static void writeToStreamOutput(
+          OzoneDataStreamOutput streamOutput,
+          InputStream body,
+          int bufferSize,
+          long length) throws IOException {
+    byte[] buffer = new byte[bufferSize];
+    ByteBuffer writeByteBuffer;
+    long total = 0;
+    do {
+      int nn = body.read(buffer);
+      if (nn == -1) {
+        break;
+      } else if (nn != bufferSize) {
+        byte[] subBuffer = new byte[nn];
+        System.arraycopy(buffer, 0, subBuffer, 0, nn);
+        writeByteBuffer = ByteBuffer.wrap(subBuffer, 0, nn);
+      } else {
+        writeByteBuffer = ByteBuffer.wrap(buffer, 0, nn);
+      }
+      streamOutput.write(writeByteBuffer, 0, nn);

Review comment:
       We should avoid buffer copying.  I wonder if we should other body type (instead of InputStream) in order to support zero buffer copying; see https://docs.oracle.com/cd/E19798-01/821-1841/6nmq2cp2b/index.html#gkccg




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