[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1354: HDDS-4151. Skip the inputstream while offset larger than zero in s3g

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

maobaolong commented on a change in pull request #1354:
URL: https://github.com/apache/hadoop-ozone/pull/1354#discussion_r477884943



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
##########
@@ -277,7 +278,8 @@ public Response get(
           try (S3WrapperInputStream s3WrapperInputStream =
               new S3WrapperInputStream(
                   key.getInputStream())) {
-            IOUtils.copyLarge(s3WrapperInputStream, dest, startOffset,
+            s3WrapperInputStream.seek(startOffset);
+            IOUtils.copyLarge(s3WrapperInputStream, dest, 0,

Review comment:
       The copyLarge related code is here
   ```java
       public static long copyLarge(final InputStream input, final OutputStream output,
                                    final long inputOffset, final long length, final byte[] buffer) throws IOException {
           if (inputOffset > 0) {
               skipFully(input, inputOffset);
           }
           if (length == 0) {
               return 0;
           }
           final int bufferLength = buffer.length;
           int bytesToRead = bufferLength;
           if (length > 0 && length < bufferLength) {
               bytesToRead = (int) length;
           }
           int read;
           long totalRead = 0;
           while (bytesToRead > 0 && EOF != (read = input.read(buffer, 0, bytesToRead))) {
               output.write(buffer, 0, read);
               totalRead += read;
               if (length > 0) { // only adjust length if not reading to the end
                   // Note the cast must work because buffer.length is an integer
                   bytesToRead = (int) Math.min(length - totalRead, bufferLength);
               }
           }
           return totalRead;
       }
   
       public static void skipFully(final InputStream input, final long toSkip) throws IOException {
           if (toSkip < 0) {
               throw new IllegalArgumentException("Bytes to skip must not be negative: " + toSkip);
           }
           final long skipped = skip(input, toSkip);
           if (skipped != toSkip) {
               throw new EOFException("Bytes to skip: " + toSkip + " actual: " + skipped);
           }
       }
   
   public static long skip(final InputStream input, final long toSkip) throws IOException {
           if (toSkip < 0) {
               throw new IllegalArgumentException("Skip count must be non-negative, actual: " + toSkip);
           }
           /*
            * N.B. no need to synchronize this because: - we don't care if the buffer is created multiple times (the data
            * is ignored) - we always use the same size buffer, so if it it is recreated it will still be OK (if the buffer
            * size were variable, we would need to synch. to ensure some other thread did not create a smaller one)
            */
           if (SKIP_BYTE_BUFFER == null) {
               SKIP_BYTE_BUFFER = new byte[SKIP_BUFFER_SIZE];
           }
           long remain = toSkip;
           while (remain > 0) {
               // See https://issues.apache.org/jira/browse/IO-203 for why we use read() rather than delegating to skip()
               final long n = input.read(SKIP_BYTE_BUFFER, 0, (int) Math.min(remain, SKIP_BUFFER_SIZE));
               if (n < 0) { // EOF
                   break;
               }
               remain -= n;
           }
           return toSkip - remain;
       }
   ```
   
   From the above code of IOUtils.java, we can know that the static method skip really read the inputstream, it is unnecessary and impact the performance, instead, using seek can set the position to the right point, Let's imaging that there are a 10GB file, when we want to read the if for offset 1GB and length 1GB, it have to read the first 1GB contents from datanode and drop it, now, we can seek to the position 1GB, and read the inputstream directly.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org