Replace Math.toIntExact with toIntBytes (#61604)

We convert longs to ints using `Math.toIntExact` in places where we're
sure there will be no overflow, but this doesn't explain the intent of
these conversions very well. This commit introduces a dedicated method
for these conversions, and adds an assertion that we never overflow.

Author: DaveCTurner

server/src/main/java/org/elasticsearch/common/unit/ByteSizeUnit.java

@@ -276,6 +276,22 @@ static long x(long d, long m, long over) {
         return d * m;
     }
 
+    /**
+     * Convert to an {@code int} number of bytes. Callers are expected to be certain this will not overflow.
+     * @throws IllegalArgumentException on overflow, unless assertions are enabled in which case it throws an {@link AssertionError}.
+     * @return The number of bytes represented as an {@code int}.
+     */
+    public final int toIntBytes(long size) {
+        final long l = toBytes(size);
+        final int i = (int) l;
+        if (i != l) {
+            final String message = "could not convert [" + size + " " + this + "] to an int";
+            assert false : message;
+            throw new IllegalArgumentException(message);
+        }
+        return i;
+    }
+
     public abstract long toBytes(long size);
 
     public abstract long toKB(long size);

server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java

@@ -52,6 +52,13 @@ public void testSimple() {
         assertThat(ByteSizeUnit.PB.toPB(10), is(new ByteSizeValue(10, ByteSizeUnit.PB).getPb()));
     }
 
+    public void testToIntBytes() {
+        assertThat(ByteSizeUnit.BYTES.toIntBytes(4), equalTo(4));
+        assertThat(ByteSizeUnit.KB.toIntBytes(4), equalTo(4096));
+        assertThat(expectThrows(AssertionError.class, () -> ByteSizeUnit.GB.toIntBytes(4)).getMessage(),
+            containsString("could not convert [4 GB] to an int"));
+    }
+
     public void testEquality() {
         String[] equalValues = new String[]{"1GB", "1024MB", "1048576KB", "1073741824B"};
         ByteSizeValue value1 = ByteSizeValue.parseBytesSizeValue(randomFrom(equalValues), "equalTest");

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotsConstants.java

@@ -7,6 +7,7 @@
 
 import org.elasticsearch.Build;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.ByteSizeUnit;
 
 import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;
 
@@ -44,4 +45,15 @@ public static boolean isSearchableSnapshotStore(Settings indexSettings) {
     public static final String CACHE_PREWARMING_THREAD_POOL_SETTING = "xpack.searchable_snapshots.cache_prewarming_thread_pool";
 
     public static final String SNAPSHOT_BLOB_CACHE_INDEX = ".snapshot-blob-cache";
+
+    /**
+     * We use {@code long} to represent offsets and lengths of files since they may be larger than 2GB, but {@code int} to represent
+     * offsets and lengths of arrays in memory which are limited to 2GB in size. We quite often need to convert from the file-based world
+     * of {@code long}s into the memory-based world of {@code int}s, knowing for certain that the result will not overflow. This method
+     * should be used to clarify that we're doing this.
+     */
+    public static int toIntBytes(long l) {
+        return ByteSizeUnit.BYTES.toIntBytes(l);
+    }
+
 }

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/blobstore/cache/BlobStoreCacheService.java

@@ -51,7 +51,7 @@ public class BlobStoreCacheService extends AbstractLifecycleComponent implements
 
     private static final Logger logger = LogManager.getLogger(BlobStoreCacheService.class);
 
-    public static final int DEFAULT_CACHED_BLOB_SIZE = Math.toIntExact(ByteSizeUnit.KB.toBytes(4L));
+    public static final int DEFAULT_CACHED_BLOB_SIZE = ByteSizeUnit.KB.toIntBytes(4);
 
     private final ClusterService clusterService;
     private final ThreadPool threadPool;

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInput.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.Channels;
 import org.elasticsearch.common.lease.Releasable;
+import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo;
 import org.elasticsearch.index.snapshots.blobstore.SlicedInputStream;
 import org.elasticsearch.index.store.BaseSearchableSnapshotIndexInput;
@@ -45,6 +46,7 @@
 import java.util.stream.IntStream;
 
 import static org.elasticsearch.index.store.checksum.ChecksumBlobContainerIndexInput.checksumToBytesArray;
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
 
 public class CachedBlobContainerIndexInput extends BaseSearchableSnapshotIndexInput {
 
@@ -56,7 +58,7 @@ public class CachedBlobContainerIndexInput extends BaseSearchableSnapshotIndexIn
     public static final IOContext CACHE_WARMING_CONTEXT = new IOContext();
 
     private static final Logger logger = LogManager.getLogger(CachedBlobContainerIndexInput.class);
-    private static final int COPY_BUFFER_SIZE = 8192;
+    private static final int COPY_BUFFER_SIZE = ByteSizeUnit.KB.toIntBytes(8);
 
     private final SearchableSnapshotDirectory directory;
     private final CacheFileReference cacheFileReference;
@@ -219,7 +221,7 @@ protected void readInternal(ByteBuffer b) throws IOException {
                         );
                         stats.addIndexCacheBytesRead(cachedBlob.length());
 
-                        final BytesRefIterator cachedBytesIterator = cachedBlob.bytes().slice(Math.toIntExact(position), length).iterator();
+                        final BytesRefIterator cachedBytesIterator = cachedBlob.bytes().slice(toIntBytes(position), length).iterator();
                         BytesRef bytesRef;
                         while ((bytesRef = cachedBytesIterator.next()) != null) {
                             b.put(bytesRef.bytes, bytesRef.offset, bytesRef.length);
@@ -235,7 +237,7 @@ protected void readInternal(ByteBuffer b) throws IOException {
                                 (channel, from, to, progressUpdater) -> {
                                     final long startTimeNanos = stats.currentTimeNanos();
                                     final BytesRefIterator iterator = cachedBlob.bytes()
-                                        .slice(Math.toIntExact(from - cachedBlob.from()), Math.toIntExact(to - from))
+                                        .slice(toIntBytes(from - cachedBlob.from()), toIntBytes(to - from))
                                         .iterator();
                                     long writePosition = from;
                                     BytesRef current;
@@ -298,7 +300,7 @@ protected void readInternal(ByteBuffer b) throws IOException {
                     final int read;
                     if ((rangeToRead.v2() - rangeToRead.v1()) < b.remaining()) {
                         final ByteBuffer duplicate = b.duplicate();
-                        duplicate.limit(duplicate.position() + Math.toIntExact(rangeToRead.v2() - rangeToRead.v1()));
+                        duplicate.limit(duplicate.position() + toIntBytes(rangeToRead.v2() - rangeToRead.v1()));
                         read = readCacheFile(channel, position, duplicate);
                         assert duplicate.position() <= b.limit();
                         b.position(duplicate.position());
@@ -311,7 +313,7 @@ protected void readInternal(ByteBuffer b) throws IOException {
                 if (indexCacheMiss != null) {
                     final Releasable onCacheFillComplete = stats.addIndexCacheFill();
                     final CompletableFuture<Integer> readFuture = cacheFile.readIfAvailableOrPending(indexCacheMiss, channel -> {
-                        final int indexCacheMissLength = Math.toIntExact(indexCacheMiss.v2() - indexCacheMiss.v1());
+                        final int indexCacheMissLength = toIntBytes(indexCacheMiss.v2() - indexCacheMiss.v1());
 
                         // We assume that we only cache small portions of blobs so that we do not need to:
                         // - use a BigArrays for allocation
@@ -373,7 +375,7 @@ private int readDirectlyIfAlreadyClosed(long position, ByteBuffer b, Exception e
             try {
                 // cache file was evicted during the range fetching, read bytes directly from blob container
                 final long length = b.remaining();
-                final byte[] copyBuffer = new byte[Math.toIntExact(Math.min(COPY_BUFFER_SIZE, length))];
+                final byte[] copyBuffer = new byte[toIntBytes(Math.min(COPY_BUFFER_SIZE, length))];
                 logger.trace(
                     () -> new ParameterizedMessage(
                         "direct reading of range [{}-{}] for cache file [{}]",
@@ -481,7 +483,7 @@ public void prefetchPart(final int part) throws IOException {
 
                 final FileChannel fc = cacheFile.getChannel();
                 assert assertFileChannelOpen(fc);
-                final byte[] copyBuffer = new byte[Math.toIntExact(Math.min(COPY_BUFFER_SIZE, rangeLength))];
+                final byte[] copyBuffer = new byte[toIntBytes(Math.min(COPY_BUFFER_SIZE, rangeLength))];
 
                 long totalBytesRead = 0L;
                 final AtomicLong totalBytesWritten = new AtomicLong();
@@ -507,8 +509,8 @@ public void prefetchPart(final int part) throws IOException {
                             (channel, start, end, progressUpdater) -> {
                                 final ByteBuffer byteBuffer = ByteBuffer.wrap(
                                     copyBuffer,
-                                    Math.toIntExact(start - readStart),
-                                    Math.toIntExact(end - start)
+                                    toIntBytes(start - readStart),
+                                    toIntBytes(end - start)
                                 );
                                 final int writtenBytes = positionalWrite(channel, start, byteBuffer);
                                 logger.trace(
@@ -557,7 +559,7 @@ private static int readSafe(
         long remaining,
         CacheFileReference cacheFileReference
     ) throws IOException {
-        final int len = (remaining < copyBuffer.length) ? Math.toIntExact(remaining) : copyBuffer.length;
+        final int len = (remaining < copyBuffer.length) ? toIntBytes(remaining) : copyBuffer.length;
         final int bytesRead = inputStream.read(copyBuffer, 0, len);
         if (bytesRead == -1) {
             throw new EOFException(
@@ -616,7 +618,7 @@ private void writeCacheFile(final FileChannel fc, final long start, final long e
         assert assertFileChannelOpen(fc);
         assert assertCurrentThreadMayWriteCacheFile();
         final long length = end - start;
-        final byte[] copyBuffer = new byte[Math.toIntExact(Math.min(COPY_BUFFER_SIZE, length))];
+        final byte[] copyBuffer = new byte[toIntBytes(Math.min(COPY_BUFFER_SIZE, length))];
         logger.trace(() -> new ParameterizedMessage("writing range [{}-{}] to cache file [{}]", start, end, cacheFileReference));
 
         long bytesCopied = 0L;
@@ -704,7 +706,7 @@ private long getRelativePositionInPart(long position) {
     }
 
     private long getLengthOfPart(long part) {
-        return fileInfo.partBytes(Math.toIntExact(part));
+        return fileInfo.partBytes(toIntBytes(part));
     }
 
     private void ensureValidPosition(long position) {

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/checksum/ChecksumBlobContainerIndexInput.java

@@ -17,6 +17,8 @@
 import java.util.Arrays;
 import java.util.Objects;
 
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
+
 /**
  * A {@link IndexInput} that can only be used to verify footer checksums.
  */
@@ -108,7 +110,7 @@ private int checksumPositionOrThrow(long pos) {
             assert false : "unexpected read or seek at position [" + pos + "] but checksum starts at [" + checksumPosition + ']';
             throw new IllegalArgumentException("Can't read or seek before footer checksum");
         }
-        return Math.toIntExact(checksum.length - (length - pos));
+        return toIntBytes(checksum.length - (length - pos));
     }
 
     private static void ensureReadOnceChecksumContext(IOContext context) {

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/direct/DirectBlobContainerIndexInput.java

@@ -25,6 +25,8 @@
 import java.util.Objects;
 import java.util.concurrent.atomic.LongAdder;
 
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
+
 /**
  * A {@link DirectBlobContainerIndexInput} instance corresponds to a single file from a Lucene directory that has been snapshotted. Because
  * large Lucene file might be split into multiple parts during the snapshot, {@link DirectBlobContainerIndexInput} requires a
@@ -110,9 +112,9 @@ protected void readInternal(ByteBuffer b) throws IOException {
                 int currentPart = Math.toIntExact(position / fileInfo.partSize().getBytes());
                 int remainingBytesInPart;
                 if (currentPart < (fileInfo.numberOfParts() - 1)) {
-                    remainingBytesInPart = Math.toIntExact(((currentPart + 1L) * fileInfo.partSize().getBytes()) - position);
+                    remainingBytesInPart = toIntBytes(((currentPart + 1L) * fileInfo.partSize().getBytes()) - position);
                 } else {
-                    remainingBytesInPart = Math.toIntExact(fileInfo.length() - position);
+                    remainingBytesInPart = toIntBytes(fileInfo.length() - position);
                 }
                 final int read = Math.min(b.remaining(), remainingBytesInPart);
                 readInternalBytes(currentPart, position % fileInfo.partSize().getBytes(), b, read);

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/CacheService.java

@@ -24,6 +24,8 @@
 import java.util.Objects;
 import java.util.function.Predicate;
 
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
+
 /**
  * {@link CacheService} maintains a cache entry for all files read from searchable snapshot directories (
  * see {@link org.elasticsearch.index.store.SearchableSnapshotDirectory})
@@ -107,7 +109,7 @@ public long getCacheSize() {
      * @return the cache range size (in bytes)
      */
     public int getRangeSize() {
-        return Math.toIntExact(rangeSize.getBytes());
+        return toIntBytes(rangeSize.getBytes());
     }
 
     public CacheFile get(final CacheKey cacheKey, final long fileLength, final Path cacheDir) throws Exception {

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/IndexInputStatsTests.java

@@ -11,10 +11,11 @@
 
 import static org.elasticsearch.index.store.IndexInputStats.SEEKING_THRESHOLD;
 import static org.elasticsearch.index.store.cache.TestUtils.assertCounter;
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
 
 public class IndexInputStatsTests extends ESTestCase {
 
-    private static LongSupplier FAKE_CLOCK = () -> {
+    private static final LongSupplier FAKE_CLOCK = () -> {
         assert false : "should not be called";
         return -1L;
     };
@@ -32,7 +33,7 @@ public void testReads() {
         for (int i = 0; i < randomIntBetween(1, 50); i++) {
             final long currentPosition = randomLongBetween(0L, inputStats.getFileLength() - 1L);
             final long previousPosition = randomBoolean() ? currentPosition : randomLongBetween(0L, inputStats.getFileLength() - 1L);
-            final int bytesRead = randomIntBetween(1, Math.toIntExact(Math.max(1L, inputStats.getFileLength() - currentPosition)));
+            final int bytesRead = randomIntBetween(1, toIntBytes(Math.max(1L, inputStats.getFileLength() - currentPosition)));
 
             inputStats.incrementBytesRead(previousPosition, currentPosition, bytesRead);
 

x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/index/store/SearchableSnapshotDirectoryStatsTests.java

@@ -28,7 +28,6 @@
 import org.elasticsearch.index.store.cache.TestUtils;
 import org.elasticsearch.index.store.cache.TestUtils.NoopBlobStoreCacheService;
 import org.elasticsearch.indices.recovery.RecoveryState;
-import org.elasticsearch.indices.recovery.RecoveryState;
 import org.elasticsearch.indices.recovery.SearchableSnapshotRecoveryState;
 import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.snapshots.SnapshotId;
@@ -52,6 +51,7 @@
 import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_CACHE_ENABLED_SETTING;
 import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_CACHE_PREWARM_ENABLED_SETTING;
 import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_UNCACHED_CHUNK_SIZE_SETTING;
+import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.toIntBytes;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
@@ -119,7 +119,7 @@ public void testCachedBytesReadsAndWrites() {
                 final IndexInputStats inputStats = directory.getStats(fileName);
                 assertThat(inputStats, notNullValue());
 
-                final byte[] result = randomReadAndSlice(input, Math.toIntExact(length));
+                final byte[] result = randomReadAndSlice(input, toIntBytes(length));
                 assertArrayEquals(fileContent, result);
 
                 final long cachedBytesWriteCount = TestUtils.numberOfRanges(length, rangeSize.getBytes());
@@ -171,7 +171,7 @@ public void testCachedBytesReadsAndWritesNoCache() {
                 final IndexInputStats inputStats = directory.getStats(fileName);
                 assertThat(inputStats, notNullValue());
 
-                final byte[] result = randomReadAndSlice(input, Math.toIntExact(length));
+                final byte[] result = randomReadAndSlice(input, toIntBytes(length));
                 assertArrayEquals(fileContent, result);
 
                 assertThat(inputStats.getCachedBytesWritten(), notNullValue());
@@ -213,7 +213,7 @@ public void testDirectBytesReadsWithCache() {
                 // read all index input sequentially as it simplifies testing
                 final byte[] readBuffer = new byte[512];
                 for (long i = 0L; i < input.length();) {
-                    int size = between(1, Math.toIntExact(Math.min(readBuffer.length, input.length() - input.getFilePointer())));
+                    int size = between(1, toIntBytes(Math.min(readBuffer.length, input.length() - input.getFilePointer())));
                     input.readBytes(readBuffer, 0, size);
                     i += size;
 
@@ -331,7 +331,7 @@ public void testReadBytesContiguously() {
 
                 // read the input input sequentially
                 for (long bytesRead = 0L; bytesRead < input.length();) {
-                    int size = between(1, Math.toIntExact(Math.min(readBuffer.length, input.length() - bytesRead)));
+                    int size = between(1, toIntBytes(Math.min(readBuffer.length, input.length() - bytesRead)));
                     input.readBytes(readBuffer, 0, size);
                     bytesRead += size;
 
@@ -381,7 +381,7 @@ public void testReadBytesNonContiguously() {
                     input.seek(randomPosition);
 
                     final byte[] readBuffer = new byte[512];
-                    int size = between(1, Math.toIntExact(Math.min(readBuffer.length, input.length() - randomPosition)));
+                    int size = between(1, toIntBytes(Math.min(readBuffer.length, input.length() - randomPosition)));
                     input.readBytes(readBuffer, 0, size);
 
                     // BufferedIndexInput tries to read as much bytes as possible