From d13a95b2680de3dcde8eaebb8f6d9dcb3a99ba7f Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 25 Oct 2021 11:21:01 +0200 Subject: [PATCH 01/19] Add Windows native method to retrieve the number of allocated bytes on disk for file --- .../bootstrap/JNAKernel32Library.java | 40 ++++++++++++++ .../elasticsearch/bootstrap/JNANatives.java | 15 ++++++ .../org/elasticsearch/bootstrap/Natives.java | 16 +++++- .../cache/common/CacheFileTests.java | 54 +++++++++++++++++++ 4 files changed, 124 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java index b6585ab6c9d90..2753fe48ed984 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java @@ -14,12 +14,15 @@ import com.sun.jna.Pointer; import com.sun.jna.Structure; import com.sun.jna.WString; +import com.sun.jna.ptr.IntByReference; import com.sun.jna.win32.StdCallLibrary; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -306,4 +309,41 @@ protected List getFieldOrder() { * @return true if the function succeeds */ native boolean SetInformationJobObject(Pointer job, int infoClass, Pointer info, int infoLength); + + /** + * Retrieves the actual number of bytes of disk storage used to store a specified file. + * + * https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getcompressedfilesizew + * + * @param lpFileName the path string + * @param lpFileSizeHigh pointer to high-order DWORD for compressed file size (or null if not needed) + * @return the low-order DWORD for compressed file siz + */ + native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); + + /** + * Returns the number of allocated bytes on disk for a given file. This method uses Kernel32 DDL native method + * {@link #GetCompressedFileSizeW(WString, IntByReference)} to retrieve the allocated size of the file. + * + * @param path the path to the file + * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size is invalid + */ + Long allocatedSizeInBytes(Path path) { + assert Files.isRegularFile(path) : path; + final WString fileName = new WString("\\\\?\\" + path); + final IntByReference lpFileSizeHigh = new IntByReference(); + + final int lpFileSizeLow = GetCompressedFileSizeW(fileName, lpFileSizeHigh); + if (lpFileSizeLow == 0xffffffff) { + final int err = Native.getLastError(); + logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); + return null; + } + + final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); + if (logger.isTraceEnabled()) { + logger.warn("native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); + } + return allocatedSize; + } } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java b/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java index de9fee12f3113..4c0ac610e6f8d 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java @@ -11,9 +11,11 @@ import com.sun.jna.Native; import com.sun.jna.Pointer; import com.sun.jna.WString; + import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; +import org.elasticsearch.core.Nullable; import org.elasticsearch.monitor.jvm.JvmInfo; import java.nio.file.Path; @@ -260,4 +262,17 @@ static void tryInstallSystemCallFilter(Path tmpFile) { } } + /** + * Returns the number of allocated bytes on disk for a given file. + * + * @param path the path o the file + * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned + */ + @Nullable + static Long allocatedSizeInBytes(Path path) { + if (Constants.WINDOWS) { + return JNAKernel32Library.getInstance().allocatedSizeInBytes(path); + } + return null; + } } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Natives.java b/server/src/main/java/org/elasticsearch/bootstrap/Natives.java index f5e94b74234c4..f781c6fa5c235 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Natives.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Natives.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.core.Nullable; import java.nio.file.Path; @@ -17,7 +18,7 @@ * The Natives class is a wrapper class that checks if the classes necessary for calling native methods are available on * startup. If they are not available, this class will avoid calling code that loads these classes. */ -final class Natives { +public final class Natives { /** no instantiation */ private Natives() {} @@ -133,4 +134,17 @@ static boolean isSystemCallFilterInstalled() { return JNANatives.LOCAL_SYSTEM_CALL_FILTER; } + /** + * Returns the number of allocated bytes on disk for a given file. + * + * @param path the path to the file + * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned + */ + @Nullable + public static Long allocatedSizeInBytes(Path path) { + if (JNA_AVAILABLE == false) { + return null; + } + return JNANatives.allocatedSizeInBytes(path); + } } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 8ad1d21885f46..5f8ee5ad7a475 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -7,7 +7,9 @@ package org.elasticsearch.xpack.searchablesnapshots.cache.common; import org.apache.lucene.store.AlreadyClosedException; +import org.apache.lucene.util.Constants; import org.apache.lucene.util.SetOnce; +import org.elasticsearch.bootstrap.Natives; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; import org.elasticsearch.core.PathUtils; @@ -20,11 +22,13 @@ import org.hamcrest.Matcher; import java.io.IOException; +import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -41,6 +45,7 @@ import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -380,6 +385,35 @@ public void testFSyncFailure() throws Exception { } } + public void testCacheFileCreatedAsSparseFile() throws Exception { + assumeTrue("This test uses a native method implemented only for Windows", Constants.WINDOWS); + + final Path file = createTempDir().resolve(UUIDs.randomBase64UUID(random())); + final CacheFile cacheFile = new CacheFile( + new CacheKey("_snap_uuid", "_snap_name", new ShardId("_name", "_uid", 0), "_filename"), + 1048576L, + file, + NOOP + ); + assertFalse(Files.exists(file)); + + final TestEvictionListener listener = new TestEvictionListener(); + cacheFile.acquire(listener); + try { + assertTrue(Files.exists(file)); + + Long sizeOnDisk = Natives.allocatedSizeInBytes(file); + assertThat(sizeOnDisk, equalTo(0L)); + + fill(cacheFile.getChannel(), Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); + + sizeOnDisk = Natives.allocatedSizeInBytes(file); + assertThat(sizeOnDisk, not(equalTo(1048576L))); + } finally { + cacheFile.release(listener); + } + } + static class TestEvictionListener implements EvictionListener { private final SetOnce evicted = new SetOnce<>(); @@ -440,4 +474,24 @@ private static FSyncTrackingFileSystemProvider setupFSyncCountingFileSystem() { PathUtilsForTesting.installMock(provider.getFileSystem(null)); return provider; } + + private static void fill(FileChannel fileChannel, int from, int to) { + final byte[] buffer = new byte[Math.min(Math.max(0, to - from), 1024)]; + Arrays.fill(buffer, (byte) 0xff); + assert fileChannel.isOpen(); + + try { + int written = 0; + int remaining = to - from; + while (remaining > 0) { + final int len = Math.min(remaining, buffer.length); + fileChannel.write(ByteBuffer.wrap(buffer, 0, len), from + written); + remaining -= len; + written += len; + } + assert written == to - from; + } catch (IOException e) { + throw new AssertionError(e); + } + } } From f4c5d772ab1369d0cca194d4a2b1c24c2af1c10b Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 25 Oct 2021 17:31:55 +0200 Subject: [PATCH 02/19] test should fail --- .../java/org/elasticsearch/bootstrap/JNAKernel32Library.java | 2 +- .../xpack/searchablesnapshots/cache/common/CacheFile.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java index 2753fe48ed984..7f68c6b1bf284 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java @@ -342,7 +342,7 @@ Long allocatedSizeInBytes(Path path) { final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); if (logger.isTraceEnabled()) { - logger.warn("native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); + logger.trace("native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); } return allocatedSize; } diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java index a827b8f3d610f..b03b87027e6b3 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java @@ -56,7 +56,7 @@ public interface ModificationListener { private static final StandardOpenOption[] CREATE_OPTIONS = new StandardOpenOption[] { StandardOpenOption.READ, StandardOpenOption.WRITE, - StandardOpenOption.CREATE_NEW, + StandardOpenOption.CREATE, StandardOpenOption.SPARSE }; private static final StandardOpenOption[] OPEN_OPTIONS = new StandardOpenOption[] { StandardOpenOption.READ, StandardOpenOption.WRITE }; From 8f95d7b8f74e549b0e8054b2d1da31500bfc2a53 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 25 Oct 2021 20:52:23 +0200 Subject: [PATCH 03/19] test should succeed --- .../xpack/searchablesnapshots/cache/common/CacheFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java index b03b87027e6b3..a827b8f3d610f 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFile.java @@ -56,7 +56,7 @@ public interface ModificationListener { private static final StandardOpenOption[] CREATE_OPTIONS = new StandardOpenOption[] { StandardOpenOption.READ, StandardOpenOption.WRITE, - StandardOpenOption.CREATE, + StandardOpenOption.CREATE_NEW, StandardOpenOption.SPARSE }; private static final StandardOpenOption[] OPEN_OPTIONS = new StandardOpenOption[] { StandardOpenOption.READ, StandardOpenOption.WRITE }; From 806f501ec895fc6fc4255de986e69d23e88a6f91 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 09:31:05 +0200 Subject: [PATCH 04/19] more docs --- .../org/elasticsearch/bootstrap/JNAKernel32Library.java | 9 ++++++--- .../searchablesnapshots/cache/common/CacheFileTests.java | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java index 7f68c6b1bf284..6bfaae264a5dc 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java @@ -322,8 +322,11 @@ protected List getFieldOrder() { native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); /** - * Returns the number of allocated bytes on disk for a given file. This method uses Kernel32 DDL native method - * {@link #GetCompressedFileSizeW(WString, IntByReference)} to retrieve the allocated size of the file. + * Retrieves the actual number of bytes of disk storage used to store a specified file. If the file is located on a volume that supports + * compression and the file is compressed, the value obtained is the compressed size of the specified file. If the file is located on a + * volume that supports sparse files and the file is a sparse file, the value obtained is the sparse size of the specified file. + * + * This method uses Win32 DLL native method {@link #GetCompressedFileSizeW(WString, IntByReference)}. * * @param path the path to the file * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size is invalid @@ -342,7 +345,7 @@ Long allocatedSizeInBytes(Path path) { final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); if (logger.isTraceEnabled()) { - logger.trace("native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); + logger.trace("executing native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); } return allocatedSize; } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 5f8ee5ad7a475..3b715a168137c 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -405,10 +405,13 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { Long sizeOnDisk = Natives.allocatedSizeInBytes(file); assertThat(sizeOnDisk, equalTo(0L)); + // write 1 byte at the last position in the cache file. + // For non sparse files, Windows would allocate the full file on disk in order to write a single byte at the end, + // making the next assertion fails. fill(cacheFile.getChannel(), Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); sizeOnDisk = Natives.allocatedSizeInBytes(file); - assertThat(sizeOnDisk, not(equalTo(1048576L))); + assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, not(equalTo(1048576L))); } finally { cacheFile.release(listener); } From f1091913ac2ac0b14f289b1141aaa5643359e5ef Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 10:46:41 +0200 Subject: [PATCH 05/19] test --- .../cache/common/CacheFileTests.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 3b715a168137c..f6fbb8dd2b39f 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -42,10 +42,11 @@ import static org.elasticsearch.xpack.searchablesnapshots.cache.common.TestUtils.randomPopulateAndReads; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -411,7 +412,16 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { fill(cacheFile.getChannel(), Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); sizeOnDisk = Natives.allocatedSizeInBytes(file); - assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, not(equalTo(1048576L))); + assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(1048576L)); + + fill(cacheFile.getChannel(), 0, Math.toIntExact(cacheFile.getLength())); + + sizeOnDisk = Natives.allocatedSizeInBytes(file); + assertThat( + "Cache file should be fully allocated on disk (maybe more given cluster/block size)", + sizeOnDisk, + greaterThanOrEqualTo(1048576L) + ); } finally { cacheFile.release(listener); } From 0ef20a0fc97788cb3b49b6b919586a0620e02db1 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 12:25:41 +0200 Subject: [PATCH 06/19] nits + fsync --- .../elasticsearch/bootstrap/JNAKernel32Library.java | 8 +++++++- .../cache/common/CacheFileTests.java | 10 +++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java index 6bfaae264a5dc..17d5363cf6f6a 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java @@ -345,7 +345,13 @@ Long allocatedSizeInBytes(Path path) { final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); if (logger.isTraceEnabled()) { - logger.trace("executing native method GetCompressedFileSizeW returned [{}] for file [{}]", allocatedSize, path); + logger.trace( + "executing native method GetCompressedFileSizeW returned [high={}, low={}, allocated={}] for file [{}]", + lpFileSizeHigh, + lpFileSizeLow, + allocatedSize, + path + ); } return allocatedSize; } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index f6fbb8dd2b39f..50a1255ebecce 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; import org.elasticsearch.core.PathUtils; import org.elasticsearch.core.PathUtilsForTesting; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; @@ -388,11 +389,12 @@ public void testFSyncFailure() throws Exception { public void testCacheFileCreatedAsSparseFile() throws Exception { assumeTrue("This test uses a native method implemented only for Windows", Constants.WINDOWS); + final long ONE_MB = 1 << 20; final Path file = createTempDir().resolve(UUIDs.randomBase64UUID(random())); final CacheFile cacheFile = new CacheFile( new CacheKey("_snap_uuid", "_snap_name", new ShardId("_name", "_uid", 0), "_filename"), - 1048576L, + ONE_MB, file, NOOP ); @@ -410,17 +412,19 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { // For non sparse files, Windows would allocate the full file on disk in order to write a single byte at the end, // making the next assertion fails. fill(cacheFile.getChannel(), Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); + IOUtils.fsync(file, false); sizeOnDisk = Natives.allocatedSizeInBytes(file); - assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(1048576L)); + assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(ONE_MB)); fill(cacheFile.getChannel(), 0, Math.toIntExact(cacheFile.getLength())); + IOUtils.fsync(file, false); sizeOnDisk = Natives.allocatedSizeInBytes(file); assertThat( "Cache file should be fully allocated on disk (maybe more given cluster/block size)", sizeOnDisk, - greaterThanOrEqualTo(1048576L) + greaterThanOrEqualTo(ONE_MB) ); } finally { cacheFile.release(listener); From 39adb834b26c1aa593add78b21f5cfe379110966 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 13:30:13 +0200 Subject: [PATCH 07/19] debug last failure @TestLogging --- .../xpack/searchablesnapshots/cache/common/CacheFileTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 50a1255ebecce..072c8ad1be0aa 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheFile.EvictionListener; import org.elasticsearch.xpack.searchablesnapshots.cache.common.TestUtils.FSyncTrackingFileSystemProvider; @@ -387,6 +388,7 @@ public void testFSyncFailure() throws Exception { } } + @TestLogging(reason = "debug failure", value = "org.elasticsearch.bootstrap:TRACE") public void testCacheFileCreatedAsSparseFile() throws Exception { assumeTrue("This test uses a native method implemented only for Windows", Constants.WINDOWS); final long ONE_MB = 1 << 20; From 15abf62d85c919c2e3f8ae3c7c5605aef6c95016 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 15:02:40 +0200 Subject: [PATCH 08/19] avoid DisableFsyncFS --- .../cache/common/CacheFileTests.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 072c8ad1be0aa..aeb6eb653a3f9 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -8,13 +8,13 @@ import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.util.Constants; +import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.SetOnce; import org.elasticsearch.bootstrap.Natives; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; import org.elasticsearch.core.PathUtils; import org.elasticsearch.core.PathUtilsForTesting; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.junit.annotations.TestLogging; @@ -53,6 +53,7 @@ import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; +@LuceneTestCase.SuppressFileSystems("DisableFsyncFS") // required by {@link testCacheFileCreatedAsSparseFile()} public class CacheFileTests extends ESTestCase { private static final CacheFile.ModificationListener NOOP = new CacheFile.ModificationListener() { @@ -65,7 +66,7 @@ public void onCacheFileDelete(CacheFile cacheFile) {} private static final CacheKey CACHE_KEY = new CacheKey("_snap_uuid", "_snap_index", new ShardId("_name", "_uuid", 0), "_filename"); - public void testGetCacheKey() throws Exception { + public void testGetCacheKey() { final Path file = createTempDir().resolve("file.new"); final CacheKey cacheKey = new CacheKey( UUIDs.randomBase64UUID(random()), @@ -405,6 +406,7 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { final TestEvictionListener listener = new TestEvictionListener(); cacheFile.acquire(listener); try { + final FileChannel fileChannel = cacheFile.getChannel(); assertTrue(Files.exists(file)); Long sizeOnDisk = Natives.allocatedSizeInBytes(file); @@ -413,14 +415,14 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { // write 1 byte at the last position in the cache file. // For non sparse files, Windows would allocate the full file on disk in order to write a single byte at the end, // making the next assertion fails. - fill(cacheFile.getChannel(), Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); - IOUtils.fsync(file, false); + fill(fileChannel, Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); + fileChannel.force(false); sizeOnDisk = Natives.allocatedSizeInBytes(file); assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(ONE_MB)); - fill(cacheFile.getChannel(), 0, Math.toIntExact(cacheFile.getLength())); - IOUtils.fsync(file, false); + fill(fileChannel, 0, Math.toIntExact(cacheFile.getLength())); + fileChannel.force(false); sizeOnDisk = Natives.allocatedSizeInBytes(file); assertThat( From d1c4f775e098dcc480c0943bc03edb9dcbcc48af Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 26 Oct 2021 18:50:01 +0200 Subject: [PATCH 09/19] remove @TestLogging --- .../xpack/searchablesnapshots/cache/common/CacheFileTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index aeb6eb653a3f9..edafb94da563d 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -17,7 +17,6 @@ import org.elasticsearch.core.PathUtilsForTesting; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.searchablesnapshots.cache.common.CacheFile.EvictionListener; import org.elasticsearch.xpack.searchablesnapshots.cache.common.TestUtils.FSyncTrackingFileSystemProvider; @@ -389,7 +388,6 @@ public void testFSyncFailure() throws Exception { } } - @TestLogging(reason = "debug failure", value = "org.elasticsearch.bootstrap:TRACE") public void testCacheFileCreatedAsSparseFile() throws Exception { assumeTrue("This test uses a native method implemented only for Windows", Constants.WINDOWS); final long ONE_MB = 1 << 20; From ee5ab761e74088570cc73f4a42d1b726e380cc7e Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 2 Nov 2021 16:41:09 +0100 Subject: [PATCH 10/19] revert changes in boostrap natives --- .../bootstrap/JNAKernel32Library.java | 49 ------------------- .../elasticsearch/bootstrap/JNANatives.java | 14 ------ .../org/elasticsearch/bootstrap/Natives.java | 16 +----- 3 files changed, 1 insertion(+), 78 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java index 8c6dc33696711..c5bdef24d6b81 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNAKernel32Library.java @@ -14,15 +14,12 @@ import com.sun.jna.Pointer; import com.sun.jna.Structure; import com.sun.jna.WString; -import com.sun.jna.ptr.IntByReference; import com.sun.jna.win32.StdCallLibrary; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -317,50 +314,4 @@ protected List getFieldOrder() { * @return true if the function succeeds */ native boolean SetInformationJobObject(Pointer job, int infoClass, Pointer info, int infoLength); - - /** - * Retrieves the actual number of bytes of disk storage used to store a specified file. - * - * https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getcompressedfilesizew - * - * @param lpFileName the path string - * @param lpFileSizeHigh pointer to high-order DWORD for compressed file size (or null if not needed) - * @return the low-order DWORD for compressed file siz - */ - native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); - - /** - * Retrieves the actual number of bytes of disk storage used to store a specified file. If the file is located on a volume that supports - * compression and the file is compressed, the value obtained is the compressed size of the specified file. If the file is located on a - * volume that supports sparse files and the file is a sparse file, the value obtained is the sparse size of the specified file. - * - * This method uses Win32 DLL native method {@link #GetCompressedFileSizeW(WString, IntByReference)}. - * - * @param path the path to the file - * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size is invalid - */ - Long allocatedSizeInBytes(Path path) { - assert Files.isRegularFile(path) : path; - final WString fileName = new WString("\\\\?\\" + path); - final IntByReference lpFileSizeHigh = new IntByReference(); - - final int lpFileSizeLow = GetCompressedFileSizeW(fileName, lpFileSizeHigh); - if (lpFileSizeLow == 0xffffffff) { - final int err = Native.getLastError(); - logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); - return null; - } - - final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); - if (logger.isTraceEnabled()) { - logger.trace( - "executing native method GetCompressedFileSizeW returned [high={}, low={}, allocated={}] for file [{}]", - lpFileSizeHigh, - lpFileSizeLow, - allocatedSize, - path - ); - } - return allocatedSize; - } } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java b/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java index ab639edf5bb3d..e81a46193c6ee 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/JNANatives.java @@ -15,7 +15,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; -import org.elasticsearch.core.Nullable; import org.elasticsearch.monitor.jvm.JvmInfo; import java.nio.file.Path; @@ -268,17 +267,4 @@ static void tryInstallSystemCallFilter(Path tmpFile) { } } - /** - * Returns the number of allocated bytes on disk for a given file. - * - * @param path the path o the file - * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned - */ - @Nullable - static Long allocatedSizeInBytes(Path path) { - if (Constants.WINDOWS) { - return JNAKernel32Library.getInstance().allocatedSizeInBytes(path); - } - return null; - } } diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Natives.java b/server/src/main/java/org/elasticsearch/bootstrap/Natives.java index f781c6fa5c235..f5e94b74234c4 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Natives.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Natives.java @@ -10,7 +10,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.core.Nullable; import java.nio.file.Path; @@ -18,7 +17,7 @@ * The Natives class is a wrapper class that checks if the classes necessary for calling native methods are available on * startup. If they are not available, this class will avoid calling code that loads these classes. */ -public final class Natives { +final class Natives { /** no instantiation */ private Natives() {} @@ -134,17 +133,4 @@ static boolean isSystemCallFilterInstalled() { return JNANatives.LOCAL_SYSTEM_CALL_FILTER; } - /** - * Returns the number of allocated bytes on disk for a given file. - * - * @param path the path to the file - * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned - */ - @Nullable - public static Long allocatedSizeInBytes(Path path) { - if (JNA_AVAILABLE == false) { - return null; - } - return JNANatives.allocatedSizeInBytes(path); - } } From 2d9275cf98a4576f2e2b714164b870bc4e72c6db Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 2 Nov 2021 16:41:39 +0100 Subject: [PATCH 11/19] add org.elasticsearch.common.filesystem --- .../elasticsearch/bootstrap/Bootstrap.java | 4 + .../common/filesystem/FileSystemNatives.java | 68 +++++++++++++ .../filesystem/WindowsFileSystemNatives.java | 97 +++++++++++++++++++ .../cache/common/CacheFileTests.java | 8 +- 4 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java create mode 100644 server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java diff --git a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index e99ce9acb81ef..7bde9dacc572e 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -20,6 +20,7 @@ import org.elasticsearch.Version; import org.elasticsearch.cli.UserException; import org.elasticsearch.common.PidFile; +import org.elasticsearch.common.filesystem.FileSystemNatives; import org.elasticsearch.common.inject.CreationException; import org.elasticsearch.common.logging.LogConfigurator; import org.elasticsearch.common.logging.Loggers; @@ -150,6 +151,9 @@ public boolean handle(int code) { // init lucene random seed. it will use /dev/urandom where available: StringHelper.randomId(); + + // init filesystem natives + FileSystemNatives.init(); } static void initializeProbes() { diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java new file mode 100644 index 0000000000000..57253fb93213f --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.common.filesystem; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.Constants; +import org.elasticsearch.core.Nullable; + +import java.nio.file.Path; + +/** + * This class provides utility methods for calling some native methods related to filesystems. + */ +public final class FileSystemNatives { + + private static final Logger logger = LogManager.getLogger(FileSystemNatives.class); + + interface Provider { + @Nullable + Long allocatedSizeInBytes(Path path); + } + + private static final Provider JNA_PROVIDER; + static { + Provider provider = null; + try { + // load one of the main JNA classes to see if the classes are available. this does not ensure that all native + // libraries are available, only the ones necessary by JNA to function + Class.forName("com.sun.jna.Native"); + if (Constants.WINDOWS) { + provider = WindowsFileSystemNatives.getInstance(); + } + } catch (ClassNotFoundException e) { + logger.warn("JNA not found. FileSystems native methods will be disabled.", e); + } catch (UnsatisfiedLinkError e) { + logger.warn("unable to load JNA native support library, FileSystems native methods will be disabled.", e); + } + JNA_PROVIDER = provider; + } + + private FileSystemNatives() {} + + public static void init() { + assert JNA_PROVIDER != null || Constants.WINDOWS == false; + } + + /** + * Returns the number of allocated bytes on disk for a given file. + * + * @param path the path to the file + * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned + */ + @Nullable + public static Long allocatedSizeInBytes(Path path) { + if (JNA_PROVIDER != null) { + return JNA_PROVIDER.allocatedSizeInBytes(path); + } + return null; + } + +} diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java new file mode 100644 index 0000000000000..931d65a9852fa --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.common.filesystem; + +import com.sun.jna.Native; +import com.sun.jna.WString; +import com.sun.jna.ptr.IntByReference; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.lucene.util.Constants; +import org.elasticsearch.core.Nullable; + +import java.nio.file.Files; +import java.nio.file.Path; + +/** + * {@link FileSystemNatives.Provider} implementation for Windows/Kernel32 + */ +final class WindowsFileSystemNatives implements FileSystemNatives.Provider { + + private static final Logger logger = LogManager.getLogger(WindowsFileSystemNatives.class); + + private static final class Holder { + private static final WindowsFileSystemNatives instance = new WindowsFileSystemNatives(); + } + + private WindowsFileSystemNatives() { + if (Constants.WINDOWS) { + try { + Native.register("kernel32"); + logger.debug("windows/Kernel32 library loaded"); + } catch (NoClassDefFoundError e) { + logger.warn("JNA not found. native methods and handlers will be disabled."); + } catch (UnsatisfiedLinkError e) { + logger.warn("unable to link Windows/Kernel32 library. native methods and handlers will be disabled."); + } + } + } + + static WindowsFileSystemNatives getInstance() { + return WindowsFileSystemNatives.Holder.instance; + } + + /** + * Retrieves the actual number of bytes of disk storage used to store a specified file. + * + * https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getcompressedfilesizew + * + * @param lpFileName the path string + * @param lpFileSizeHigh pointer to high-order DWORD for compressed file size (or null if not needed) + * @return the low-order DWORD for compressed file siz + */ + native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); + + /** + * Retrieves the actual number of bytes of disk storage used to store a specified file. If the file is located on a volume that supports + * compression and the file is compressed, the value obtained is the compressed size of the specified file. If the file is located on a + * volume that supports sparse files and the file is a sparse file, the value obtained is the sparse size of the specified file. + * + * This method uses Win32 DLL native method {@link #GetCompressedFileSizeW(WString, IntByReference)}. + * + * @param path the path to the file + * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size is invalid + */ + @Nullable + public Long allocatedSizeInBytes(Path path) { + assert Files.isRegularFile(path) : path; + final WString fileName = new WString("\\\\?\\" + path); + final IntByReference lpFileSizeHigh = new IntByReference(); + + final int lpFileSizeLow = GetCompressedFileSizeW(fileName, lpFileSizeHigh); + if (lpFileSizeLow == 0xffffffff) { + final int err = Native.getLastError(); + logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); + return null; + } + + final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); + if (logger.isTraceEnabled()) { + logger.trace( + "executing native method GetCompressedFileSizeW returned [high={}, low={}, allocated={}] for file [{}]", + lpFileSizeHigh, + lpFileSizeLow, + allocatedSize, + path + ); + } + return allocatedSize; + } +} diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index edafb94da563d..5d653bb4f91ae 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -10,8 +10,8 @@ import org.apache.lucene.util.Constants; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.SetOnce; -import org.elasticsearch.bootstrap.Natives; import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.filesystem.FileSystemNatives; import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue; import org.elasticsearch.core.PathUtils; import org.elasticsearch.core.PathUtilsForTesting; @@ -407,7 +407,7 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { final FileChannel fileChannel = cacheFile.getChannel(); assertTrue(Files.exists(file)); - Long sizeOnDisk = Natives.allocatedSizeInBytes(file); + Long sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); assertThat(sizeOnDisk, equalTo(0L)); // write 1 byte at the last position in the cache file. @@ -416,13 +416,13 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { fill(fileChannel, Math.toIntExact(cacheFile.getLength() - 1L), Math.toIntExact(cacheFile.getLength())); fileChannel.force(false); - sizeOnDisk = Natives.allocatedSizeInBytes(file); + sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(ONE_MB)); fill(fileChannel, 0, Math.toIntExact(cacheFile.getLength())); fileChannel.force(false); - sizeOnDisk = Natives.allocatedSizeInBytes(file); + sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); assertThat( "Cache file should be fully allocated on disk (maybe more given cluster/block size)", sizeOnDisk, From c6ba764897309bf15fe2fa30e52a71bfa6417934 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 4 Nov 2021 12:41:51 +0100 Subject: [PATCH 12/19] feedback --- .../common/filesystem/FileSystemNatives.java | 30 +++++++++++-------- .../filesystem/WindowsFileSystemNatives.java | 19 +++++------- .../cache/common/CacheFileTests.java | 12 +++++--- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java index 57253fb93213f..bab2636e29321 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java @@ -11,9 +11,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; -import org.elasticsearch.core.Nullable; import java.nio.file.Path; +import java.util.OptionalLong; /** * This class provides utility methods for calling some native methods related to filesystems. @@ -23,13 +23,19 @@ public final class FileSystemNatives { private static final Logger logger = LogManager.getLogger(FileSystemNatives.class); interface Provider { - @Nullable - Long allocatedSizeInBytes(Path path); + OptionalLong allocatedSizeInBytes(Path path); + } + + private static final class NoopFileSystemNativesProvider implements Provider { + @Override + public OptionalLong allocatedSizeInBytes(Path path) { + return OptionalLong.empty(); + } } private static final Provider JNA_PROVIDER; static { - Provider provider = null; + Provider provider = new NoopFileSystemNativesProvider(); try { // load one of the main JNA classes to see if the classes are available. this does not ensure that all native // libraries are available, only the ones necessary by JNA to function @@ -41,28 +47,26 @@ interface Provider { logger.warn("JNA not found. FileSystems native methods will be disabled.", e); } catch (UnsatisfiedLinkError e) { logger.warn("unable to load JNA native support library, FileSystems native methods will be disabled.", e); + } finally { + JNA_PROVIDER = provider; } - JNA_PROVIDER = provider; } private FileSystemNatives() {} public static void init() { - assert JNA_PROVIDER != null || Constants.WINDOWS == false; + assert JNA_PROVIDER != null; } /** * Returns the number of allocated bytes on disk for a given file. * * @param path the path to the file - * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size could not be returned + * @return an {@link OptionalLong} that contains the number of allocated bytes on disk for the file. The optional is empty is the + * allocated size of the file failed be retrieved using native methods */ - @Nullable - public static Long allocatedSizeInBytes(Path path) { - if (JNA_PROVIDER != null) { - return JNA_PROVIDER.allocatedSizeInBytes(path); - } - return null; + public static OptionalLong allocatedSizeInBytes(Path path) { + return JNA_PROVIDER.allocatedSizeInBytes(path); } } diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java index 931d65a9852fa..7345000382096 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -15,10 +15,10 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.Constants; -import org.elasticsearch.core.Nullable; import java.nio.file.Files; import java.nio.file.Path; +import java.util.OptionalLong; /** * {@link FileSystemNatives.Provider} implementation for Windows/Kernel32 @@ -27,9 +27,7 @@ final class WindowsFileSystemNatives implements FileSystemNatives.Provider { private static final Logger logger = LogManager.getLogger(WindowsFileSystemNatives.class); - private static final class Holder { - private static final WindowsFileSystemNatives instance = new WindowsFileSystemNatives(); - } + private static final WindowsFileSystemNatives INSTANCE = new WindowsFileSystemNatives(); private WindowsFileSystemNatives() { if (Constants.WINDOWS) { @@ -45,7 +43,7 @@ private WindowsFileSystemNatives() { } static WindowsFileSystemNatives getInstance() { - return WindowsFileSystemNatives.Holder.instance; + return INSTANCE; } /** @@ -57,7 +55,7 @@ static WindowsFileSystemNatives getInstance() { * @param lpFileSizeHigh pointer to high-order DWORD for compressed file size (or null if not needed) * @return the low-order DWORD for compressed file siz */ - native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); + private native int GetCompressedFileSizeW(WString lpFileName, IntByReference lpFileSizeHigh); /** * Retrieves the actual number of bytes of disk storage used to store a specified file. If the file is located on a volume that supports @@ -67,10 +65,9 @@ static WindowsFileSystemNatives getInstance() { * This method uses Win32 DLL native method {@link #GetCompressedFileSizeW(WString, IntByReference)}. * * @param path the path to the file - * @return the number of allocated bytes on disk for the file or {@code null} if the allocated size is invalid + * @return an {@link OptionalLong} that contains the number of allocated bytes on disk for the file, or empty if the size is invalid */ - @Nullable - public Long allocatedSizeInBytes(Path path) { + public OptionalLong allocatedSizeInBytes(Path path) { assert Files.isRegularFile(path) : path; final WString fileName = new WString("\\\\?\\" + path); final IntByReference lpFileSizeHigh = new IntByReference(); @@ -79,7 +76,7 @@ public Long allocatedSizeInBytes(Path path) { if (lpFileSizeLow == 0xffffffff) { final int err = Native.getLastError(); logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); - return null; + return OptionalLong.empty(); } final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); @@ -92,6 +89,6 @@ public Long allocatedSizeInBytes(Path path) { path ); } - return allocatedSize; + return OptionalLong.of(allocatedSize); } } diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 5d653bb4f91ae..7fa6d5a5d33a7 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.OptionalLong; import java.util.Set; import java.util.SortedSet; import java.util.concurrent.ExecutionException; @@ -407,8 +408,9 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { final FileChannel fileChannel = cacheFile.getChannel(); assertTrue(Files.exists(file)); - Long sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); - assertThat(sizeOnDisk, equalTo(0L)); + OptionalLong sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); + assertTrue(sizeOnDisk.isPresent()); + assertThat(sizeOnDisk.getAsLong(), equalTo(0L)); // write 1 byte at the last position in the cache file. // For non sparse files, Windows would allocate the full file on disk in order to write a single byte at the end, @@ -417,15 +419,17 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { fileChannel.force(false); sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); - assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk, lessThan(ONE_MB)); + assertTrue(sizeOnDisk.isPresent()); + assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk.getAsLong(), lessThan(ONE_MB)); fill(fileChannel, 0, Math.toIntExact(cacheFile.getLength())); fileChannel.force(false); sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); + assertTrue(sizeOnDisk.isPresent()); assertThat( "Cache file should be fully allocated on disk (maybe more given cluster/block size)", - sizeOnDisk, + sizeOnDisk.getAsLong(), greaterThanOrEqualTo(ONE_MB) ); } finally { From e5a0e2c72ec61ebc46d91ea247d391b74b714dd7 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 4 Nov 2021 16:03:16 +0100 Subject: [PATCH 13/19] more --- .../common/filesystem/FileSystemNatives.java | 6 +++--- .../filesystem/WindowsFileSystemNatives.java | 16 +++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java index bab2636e29321..08476576ee26a 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java @@ -44,9 +44,9 @@ public OptionalLong allocatedSizeInBytes(Path path) { provider = WindowsFileSystemNatives.getInstance(); } } catch (ClassNotFoundException e) { - logger.warn("JNA not found. FileSystems native methods will be disabled.", e); - } catch (UnsatisfiedLinkError e) { - logger.warn("unable to load JNA native support library, FileSystems native methods will be disabled.", e); + logger.warn("JNA not found. FileSystemNatives methods will be disabled.", e); + } catch (LinkageError e) { + logger.warn("unable to load JNA native support library, FileSystemNatives methods will be disabled.", e); } finally { JNA_PROVIDER = provider; } diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java index 7345000382096..14d969bfc14e8 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -30,15 +30,13 @@ final class WindowsFileSystemNatives implements FileSystemNatives.Provider { private static final WindowsFileSystemNatives INSTANCE = new WindowsFileSystemNatives(); private WindowsFileSystemNatives() { - if (Constants.WINDOWS) { - try { - Native.register("kernel32"); - logger.debug("windows/Kernel32 library loaded"); - } catch (NoClassDefFoundError e) { - logger.warn("JNA not found. native methods and handlers will be disabled."); - } catch (UnsatisfiedLinkError e) { - logger.warn("unable to link Windows/Kernel32 library. native methods and handlers will be disabled."); - } + assert Constants.WINDOWS : Constants.OS_NAME; + try { + Native.register("kernel32"); + logger.debug("windows/Kernel32 library loaded"); + } catch (LinkageError e) { + logger.warn("unable to link Windows/Kernel32 library. native methods and handlers will be disabled.", e); + throw e; } } From 9258da1a4fe7cbaa6f5bbbbfd1ee303a383bf4b4 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 11:38:13 +0100 Subject: [PATCH 14/19] static method to init static member --- .../common/filesystem/FileSystemNatives.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java index 08476576ee26a..08c8d3ac83687 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java @@ -33,23 +33,22 @@ public OptionalLong allocatedSizeInBytes(Path path) { } } - private static final Provider JNA_PROVIDER; - static { - Provider provider = new NoopFileSystemNativesProvider(); + private static final Provider JNA_PROVIDER = loadJnaProvider(); + + private static Provider loadJnaProvider() { try { // load one of the main JNA classes to see if the classes are available. this does not ensure that all native // libraries are available, only the ones necessary by JNA to function Class.forName("com.sun.jna.Native"); if (Constants.WINDOWS) { - provider = WindowsFileSystemNatives.getInstance(); + return WindowsFileSystemNatives.getInstance(); } } catch (ClassNotFoundException e) { logger.warn("JNA not found. FileSystemNatives methods will be disabled.", e); } catch (LinkageError e) { logger.warn("unable to load JNA native support library, FileSystemNatives methods will be disabled.", e); - } finally { - JNA_PROVIDER = provider; } + return new NoopFileSystemNativesProvider(); } private FileSystemNatives() {} From e1aff78325491898c47512ebf27bf58eda5af6cb Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 11:40:18 +0100 Subject: [PATCH 15/19] @FunctionalInterface --- .../common/filesystem/FileSystemNatives.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java index 08c8d3ac83687..e1d42ece2b05b 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/FileSystemNatives.java @@ -22,17 +22,12 @@ public final class FileSystemNatives { private static final Logger logger = LogManager.getLogger(FileSystemNatives.class); + @FunctionalInterface interface Provider { OptionalLong allocatedSizeInBytes(Path path); } - private static final class NoopFileSystemNativesProvider implements Provider { - @Override - public OptionalLong allocatedSizeInBytes(Path path) { - return OptionalLong.empty(); - } - } - + private static final Provider NOOP_FILE_SYSTEM_NATIVES_PROVIDER = path -> OptionalLong.empty(); private static final Provider JNA_PROVIDER = loadJnaProvider(); private static Provider loadJnaProvider() { @@ -48,7 +43,7 @@ private static Provider loadJnaProvider() { } catch (LinkageError e) { logger.warn("unable to load JNA native support library, FileSystemNatives methods will be disabled.", e); } - return new NoopFileSystemNativesProvider(); + return NOOP_FILE_SYSTEM_NATIVES_PROVIDER; } private FileSystemNatives() {} From c00295fea4e92a5ecddf672f60d34d6a3ff3fa16 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 11:41:01 +0100 Subject: [PATCH 16/19] oneMb --- .../searchablesnapshots/cache/common/CacheFileTests.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java index 7fa6d5a5d33a7..70cd653a6b540 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/common/CacheFileTests.java @@ -391,12 +391,12 @@ public void testFSyncFailure() throws Exception { public void testCacheFileCreatedAsSparseFile() throws Exception { assumeTrue("This test uses a native method implemented only for Windows", Constants.WINDOWS); - final long ONE_MB = 1 << 20; + final long oneMb = 1 << 20; final Path file = createTempDir().resolve(UUIDs.randomBase64UUID(random())); final CacheFile cacheFile = new CacheFile( new CacheKey("_snap_uuid", "_snap_name", new ShardId("_name", "_uid", 0), "_filename"), - ONE_MB, + oneMb, file, NOOP ); @@ -420,7 +420,7 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { sizeOnDisk = FileSystemNatives.allocatedSizeInBytes(file); assertTrue(sizeOnDisk.isPresent()); - assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk.getAsLong(), lessThan(ONE_MB)); + assertThat("Cache file should be sparse and not fully allocated on disk", sizeOnDisk.getAsLong(), lessThan(oneMb)); fill(fileChannel, 0, Math.toIntExact(cacheFile.getLength())); fileChannel.force(false); @@ -430,7 +430,7 @@ public void testCacheFileCreatedAsSparseFile() throws Exception { assertThat( "Cache file should be fully allocated on disk (maybe more given cluster/block size)", sizeOnDisk.getAsLong(), - greaterThanOrEqualTo(ONE_MB) + greaterThanOrEqualTo(oneMb) ); } finally { cacheFile.release(listener); From 853a9de8ba781478555153b769f47a95732463cc Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 11:46:55 +0100 Subject: [PATCH 17/19] comment --- .../common/filesystem/WindowsFileSystemNatives.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java index 14d969bfc14e8..93dcd4ac50c77 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -77,7 +77,8 @@ public OptionalLong allocatedSizeInBytes(Path path) { return OptionalLong.empty(); } - final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << 32) | (lpFileSizeLow & 0xffffffffL); + // convert lpFileSizeLow to unsigned long and combine with signed/shifted lpFileSizeHigh + final long allocatedSize = (((long) lpFileSizeHigh.getValue()) << Integer.SIZE) | Integer.toUnsignedLong(lpFileSizeLow); if (logger.isTraceEnabled()) { logger.trace( "executing native method GetCompressedFileSizeW returned [high={}, low={}, allocated={}] for file [{}]", From 5928b399d7540fb87f11cf024566ac0027616a99 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 11:51:00 +0100 Subject: [PATCH 18/19] INVALID_FILE_SIZE --- .../common/filesystem/WindowsFileSystemNatives.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java index 93dcd4ac50c77..7b4e418fb8bc3 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -29,6 +29,8 @@ final class WindowsFileSystemNatives implements FileSystemNatives.Provider { private static final WindowsFileSystemNatives INSTANCE = new WindowsFileSystemNatives(); + private static final int INVALID_FILE_SIZE = -1; + private WindowsFileSystemNatives() { assert Constants.WINDOWS : Constants.OS_NAME; try { @@ -71,7 +73,7 @@ public OptionalLong allocatedSizeInBytes(Path path) { final IntByReference lpFileSizeHigh = new IntByReference(); final int lpFileSizeLow = GetCompressedFileSizeW(fileName, lpFileSizeHigh); - if (lpFileSizeLow == 0xffffffff) { + if (lpFileSizeLow == INVALID_FILE_SIZE) { final int err = Native.getLastError(); logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); return OptionalLong.empty(); From 3357e8fb51ae93352c04b1a5620b0194228f6151 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 Nov 2021 12:24:02 +0100 Subject: [PATCH 19/19] NO_ERROR --- .../common/filesystem/WindowsFileSystemNatives.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java index 7b4e418fb8bc3..4fe219bfc774d 100644 --- a/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java +++ b/server/src/main/java/org/elasticsearch/common/filesystem/WindowsFileSystemNatives.java @@ -30,6 +30,7 @@ final class WindowsFileSystemNatives implements FileSystemNatives.Provider { private static final WindowsFileSystemNatives INSTANCE = new WindowsFileSystemNatives(); private static final int INVALID_FILE_SIZE = -1; + private static final int NO_ERROR = 0; private WindowsFileSystemNatives() { assert Constants.WINDOWS : Constants.OS_NAME; @@ -75,8 +76,10 @@ public OptionalLong allocatedSizeInBytes(Path path) { final int lpFileSizeLow = GetCompressedFileSizeW(fileName, lpFileSizeHigh); if (lpFileSizeLow == INVALID_FILE_SIZE) { final int err = Native.getLastError(); - logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); - return OptionalLong.empty(); + if (err != NO_ERROR) { + logger.warn("error [{}] when executing native method GetCompressedFileSizeW for file [{}]", err, path); + return OptionalLong.empty(); + } } // convert lpFileSizeLow to unsigned long and combine with signed/shifted lpFileSizeHigh