From ac829edc55db84eef2d0fd31286b736dfd68f48b Mon Sep 17 00:00:00 2001
From: Tim Brooks <tim@timbrooks.org>
Date: Thu, 25 May 2023 10:11:27 -0600
Subject: [PATCH] Enable skip methods on retrying inputstreams (#96337)

Currently we have a number of input streams that specifically override
the skip() method disabling the ability to skip bytes. In each case the
skip implementation works as we have properly implemented the
read(byte[]) methods used to discard bytes. However, we appear to have
disabled it as it would be possible to retry from the end of a skip if
there is a failure in the middle. At this time, that optimization is not
really necessary, however, we sporadically used skip so it would be nice
for the IS to support the method. This commit enables the super.skip()
and adds a comment about future optimizations.
---
 .../elasticsearch/repositories/azure/AzureBlobStore.java    | 5 -----
 .../gcs/GoogleCloudStorageRetryingInputStream.java          | 6 ++++--
 .../repositories/s3/S3RetryingInputStream.java              | 6 ++++--
 .../common/blobstore/url/http/RetryingHttpInputStream.java  | 6 ++++--
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/modules/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java b/modules/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java
index 62ded398c11f..5f1d22fa9ecc 100644
--- a/modules/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java
+++ b/modules/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java
@@ -765,11 +765,6 @@ public class AzureBlobStore implements BlobStore {
             }
         }
 
-        @Override
-        public long skip(long n) {
-            throw new UnsupportedOperationException("skip is not supported");
-        }
-
         private void releaseByteBuf(ByteBuf buf) {
             ReferenceCountUtil.safeRelease(buf);
             this.byteBuf = null;
diff --git a/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRetryingInputStream.java b/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRetryingInputStream.java
index 36c05191089b..025873878975 100644
--- a/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRetryingInputStream.java
+++ b/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageRetryingInputStream.java
@@ -246,8 +246,10 @@ class GoogleCloudStorageRetryingInputStream extends InputStream {
     }
 
     @Override
-    public long skip(long n) {
-        throw new UnsupportedOperationException("GoogleCloudStorageRetryingInputStream does not support seeking");
+    public long skip(long n) throws IOException {
+        // This could be optimized on a failure by re-opening stream directly to the preferred location. However, it is rarely called,
+        // so for now we will rely on the default implementation which just discards bytes by reading.
+        return super.skip(n);
     }
 
     @Override
diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java
index 93e218c70048..7885e36c3c29 100644
--- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java
+++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RetryingInputStream.java
@@ -225,8 +225,10 @@ class S3RetryingInputStream extends InputStream {
     }
 
     @Override
-    public long skip(long n) {
-        throw new UnsupportedOperationException("S3RetryingInputStream does not support seeking");
+    public long skip(long n) throws IOException {
+        // This could be optimized on a failure by re-opening stream directly to the preferred location. However, it is rarely called,
+        // so for now we will rely on the default implementation which just discards bytes by reading.
+        return super.skip(n);
     }
 
     @Override
diff --git a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/http/RetryingHttpInputStream.java b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/http/RetryingHttpInputStream.java
index e2644da2182d..70aaf9864d56 100644
--- a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/http/RetryingHttpInputStream.java
+++ b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/http/RetryingHttpInputStream.java
@@ -112,8 +112,10 @@ class RetryingHttpInputStream extends InputStream {
     }
 
     @Override
-    public long skip(long n) {
-        throw new UnsupportedOperationException("RetryingHttpInputStream does not support seeking");
+    public long skip(long n) throws IOException {
+        // This could be optimized on a failure by re-opening stream directly to the preferred location. However, it is rarely called,
+        // so for now we will rely on the default implementation which just discards bytes by reading.
+        return super.skip(n);
     }
 
     @Override