Simplify range calculations in searchable snapshots (#96154)

We had the same complicated logic for calculating the write range in
two places and then once more in stateless.
We can just do a proper method that computes the complete write range
and also do away with all the complexity of the default range calculations
in the other use cases of the computeRange method by inlining the relevant logic.

-> much clearer why the math is the way it is and some code saved IMO.

Author: original-brownbear

x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/BlobCacheUtils.java

@@ -44,10 +44,11 @@ public static void ensureSeek(long pos, IndexInput input) throws IOException {
         }
     }
 
-    public static ByteRange computeRange(long rangeSize, long position, long length) {
-        long start = (position / rangeSize) * rangeSize;
-        long end = Math.min(start + rangeSize, length);
-        return ByteRange.of(start, end);
+    public static ByteRange computeRange(long rangeSize, long position, long size, long blobLength) {
+        return ByteRange.of(
+            (position / rangeSize) * rangeSize,
+            Math.min((((position + size - 1) / rangeSize) + 1) * rangeSize, blobLength)
+        );
     }
 
     public static void ensureSlice(String sliceName, long sliceOffset, long sliceLength, IndexInput input) {

x-pack/plugin/blob-cache/src/main/java/org/elasticsearch/blobcache/common/ByteRange.java

@@ -7,8 +7,6 @@
 
 package org.elasticsearch.blobcache.common;
 
-import org.elasticsearch.core.Nullable;
-
 public final class ByteRange implements Comparable<ByteRange> {
 
     public static final ByteRange EMPTY = new ByteRange(0L, 0L);
@@ -28,24 +26,6 @@ private ByteRange(long start, long end) {
         assert end >= start : "End must be greater or equal to start but saw [" + start + "][" + start + "]";
     }
 
-    /**
-     * Computes the smallest range that contains both this instance as well as the given {@code other} range.
-     *
-     * @param other other range or {@code null} in which case this instance is returned
-     */
-    public ByteRange minEnvelope(@Nullable ByteRange other) {
-        if (other == null) {
-            return this;
-        }
-        if (other.isSubRangeOf(this)) {
-            return this;
-        }
-        if (this.isSubRangeOf(other)) {
-            return other;
-        }
-        return of(Math.min(start, other.start), Math.max(end, other.end));
-    }
-
     public long start() {
         return start;
     }

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/store/input/CachedBlobContainerIndexInput.java

@@ -10,6 +10,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.store.IOContext;
+import org.elasticsearch.blobcache.BlobCacheUtils;
 import org.elasticsearch.blobcache.common.ByteRange;
 import org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo;
 import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheFile;
@@ -98,13 +99,6 @@ private CachedBlobContainerIndexInput(
         );
     }
 
-    @Override
-    protected long getDefaultRangeSize() {
-        return (context != CACHE_WARMING_CONTEXT) ? (directory.isRecoveryFinalized() ? defaultRangeSize : recoveryRangeSize)
-            : fileInfo.numberOfParts() == 1 ? Long.MAX_VALUE
-            : fileInfo.partSize().getBytes();
-    }
-
     @Override
     protected void readWithoutBlobCache(ByteBuffer b) throws Exception {
         ensureContext(ctx -> ctx != CACHE_WARMING_CONTEXT);
@@ -113,11 +107,12 @@ protected void readWithoutBlobCache(ByteBuffer b) throws Exception {
 
         final CacheFile cacheFile = cacheFileReference.get();
 
-        final ByteRange startRangeToWrite = computeRange(position);
-        final ByteRange endRangeToWrite = computeRange(position + length - 1);
-        assert startRangeToWrite.end() <= endRangeToWrite.end() : startRangeToWrite + " vs " + endRangeToWrite;
-        final ByteRange rangeToWrite = startRangeToWrite.minEnvelope(endRangeToWrite);
-
+        final ByteRange rangeToWrite = BlobCacheUtils.computeRange(
+            directory.isRecoveryFinalized() ? defaultRangeSize : recoveryRangeSize,
+            position,
+            length,
+            fileInfo.length()
+        );
         final ByteRange rangeToRead = ByteRange.of(position, position + length);
         assert rangeToRead.isSubRangeOf(rangeToWrite) : rangeToRead + " vs " + rangeToWrite;
         assert rangeToRead.length() == b.remaining() : b.remaining() + " vs " + rangeToRead;
@@ -144,7 +139,14 @@ public long prefetchPart(final int part, Supplier<Boolean> isCancelled) throws I
         if (isCancelled.get()) {
             return -1L;
         }
-        final ByteRange partRange = computeRange(IntStream.range(0, part).mapToLong(fileInfo::partBytes).sum());
+        final ByteRange partRange;
+        if (fileInfo.numberOfParts() == 1) {
+            partRange = ByteRange.of(0, fileInfo.length());
+        } else {
+            long rangeSize = fileInfo.partSize().getBytes();
+            long rangeStart = (IntStream.range(0, part).mapToLong(fileInfo::partBytes).sum() / rangeSize) * rangeSize;
+            partRange = ByteRange.of(rangeStart, Math.min(rangeStart + rangeSize, fileInfo.length()));
+        }
         assert assertRangeIsAlignedWithPart(partRange);
 
         try {

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/store/input/FrozenIndexInput.java

@@ -10,6 +10,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.store.IOContext;
+import org.elasticsearch.blobcache.BlobCacheUtils;
 import org.elasticsearch.blobcache.common.ByteBufferReference;
 import org.elasticsearch.blobcache.common.ByteRange;
 import org.elasticsearch.blobcache.shared.SharedBlobCacheService;
@@ -93,11 +94,6 @@ private FrozenIndexInput(
         this.cacheFile = cacheFile;
     }
 
-    @Override
-    protected long getDefaultRangeSize() {
-        return directory.isRecoveryFinalized() ? defaultRangeSize : recoveryRangeSize;
-    }
-
     @Override
     protected void readWithoutBlobCache(ByteBuffer b) throws Exception {
         final long position = getAbsolutePosition();
@@ -108,11 +104,12 @@ protected void readWithoutBlobCache(ByteBuffer b) throws Exception {
         final ByteBufferReference byteBufferReference = new ByteBufferReference(b);
         logger.trace("readInternal: read [{}-{}] from [{}]", position, position + length, this);
         try {
-            final ByteRange startRangeToWrite = computeRange(position);
-            final ByteRange endRangeToWrite = computeRange(position + length - 1);
-            assert startRangeToWrite.end() <= endRangeToWrite.end() : startRangeToWrite + " vs " + endRangeToWrite;
-            final ByteRange rangeToWrite = startRangeToWrite.minEnvelope(endRangeToWrite);
-
+            final ByteRange rangeToWrite = BlobCacheUtils.computeRange(
+                directory.isRecoveryFinalized() ? defaultRangeSize : recoveryRangeSize,
+                position,
+                length,
+                fileInfo.length()
+            );
             assert rangeToWrite.start() <= position && position + length <= rangeToWrite.end()
                 : "[" + position + "-" + (position + length) + "] vs " + rangeToWrite;
             final ByteRange rangeToRead = ByteRange.of(position, position + length);

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/store/input/MetadataCachingIndexInput.java

@@ -370,12 +370,6 @@ protected int readDirectlyIfAlreadyClosed(long position, ByteBuffer b, Exception
         throw new IOException("failed to read data from cache", e);
     }
 
-    protected abstract long getDefaultRangeSize();
-
-    protected ByteRange computeRange(long position) {
-        return BlobCacheUtils.computeRange(getDefaultRangeSize(), position, fileInfo.length());
-    }
-
     @Override
     protected void seekInternal(long pos) throws IOException {
         BlobCacheUtils.ensureSeek(pos, this);