This is an automated email from the ASF dual-hosted git repository. mmerli pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push: new fbd18aeeca Use JNI directly for posix_fadvise (#3824) fbd18aeeca is described below commit fbd18aeecaf8089c4a15676fea0dff3b32615293 Author: Matteo Merli <mme...@apache.org> AuthorDate: Tue Feb 28 17:52:40 2023 -0800 Use JNI directly for posix_fadvise (#3824) * Use JNI directly for posix_fadvise * Fixed checkstyle * fixed checkstyle * Fixed jni function name --- .../src/main/resources/LICENSE-all.bin.txt | 2 - .../src/main/resources/LICENSE-bkctl.bin.txt | 2 - .../src/main/resources/LICENSE-server.bin.txt | 2 - bookkeeper-server/pom.xml | 4 -- .../apache/bookkeeper/bookie/JournalChannel.java | 6 +-- .../util/{NativeIO.java => PageCacheUtil.java} | 62 ++++++++-------------- .../bookkeeper/common/util/nativeio/NativeIO.java | 6 +++ .../common/util/nativeio/NativeIOImpl.java | 5 ++ .../common/util/nativeio/NativeIOJni.java | 2 + .../src/main/native-io-jni/cpp/native_io_jni.c | 21 ++++++++ pom.xml | 8 --- shaded/distributedlog-core-shaded/pom.xml | 6 --- 12 files changed, 60 insertions(+), 66 deletions(-) diff --git a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt index 5e917f13bc..b66bc8b749 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt @@ -258,7 +258,6 @@ Apache Software License, Version 2. - lib/org.apache.logging.log4j-log4j-api-2.18.0.jar [17] - lib/org.apache.logging.log4j-log4j-core-2.18.0.jar [17] - lib/org.apache.logging.log4j-log4j-slf4j-impl-2.18.0.jar [17] -- lib/net.java.dev.jna-jna-5.12.1.jar [18] - lib/org.apache.commons-commons-collections4-4.1.jar [19] - lib/org.apache.commons-commons-lang3-3.6.jar [20] - lib/org.apache.zookeeper-zookeeper-3.8.1.jar [21] @@ -339,7 +338,6 @@ Apache Software License, Version 2. [15] Source available at https://github.com/eclipse/vert.x/tree/4.3.2 [16] Source available at https://github.com/vert-x3/vertx-web/tree/4.3.2 [17] Source available at https://github.com/apache/logging-log4j2/tree/rel/2.18.0 -[18] Source available at https://github.com/java-native-access/jna/tree/5.12.1 [19] Source available at https://github.com/apache/commons-collections/tree/collections-4.1 [20] Source available at https://github.com/apache/commons-lang/tree/LANG_3_6 [21] Source available at https://github.com/apache/zookeeper/tree/release-3.8.0 diff --git a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt index 102daaa7f7..013d46207a 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-bkctl.bin.txt @@ -243,7 +243,6 @@ Apache Software License, Version 2. - lib/org.apache.logging.log4j-log4j-api-2.18.0.jar [16] - lib/org.apache.logging.log4j-log4j-core-2.18.0.jar [16] - lib/org.apache.logging.log4j-log4j-slf4j-impl-2.18.0.jar [16] -- lib/net.java.dev.jna-jna-5.12.1.jar [17] - lib/org.apache.commons-commons-collections4-4.1.jar [18] - lib/org.apache.commons-commons-lang3-3.6.jar [19] - lib/org.apache.zookeeper-zookeeper-3.8.1.jar [20] @@ -305,7 +304,6 @@ Apache Software License, Version 2. [10] Source available at https://github.com/apache/commons-logging/tree/commons-logging-1.1.1 [11] Source available at https://github.com/netty/netty/tree/netty-4.1.89.Final [16] Source available at https://github.com/apache/logging-log4j2/tree/rel/2.18.0 -[17] Source available at https://github.com/java-native-access/jna/tree/5.12.1 [18] Source available at https://github.com/apache/commons-collections/tree/collections-4.1 [19] Source available at https://github.com/apache/commons-lang/tree/LANG_3_6 [20] Source available at https://github.com/apache/zookeeper/tree/release-3.8.0 diff --git a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt index 23018b504b..cde305a40b 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt @@ -258,7 +258,6 @@ Apache Software License, Version 2. - lib/org.apache.logging.log4j-log4j-api-2.18.0.jar [17] - lib/org.apache.logging.log4j-log4j-core-2.18.0.jar [17] - lib/org.apache.logging.log4j-log4j-slf4j-impl-2.18.0.jar [17] -- lib/net.java.dev.jna-jna-5.12.1.jar [18] - lib/org.apache.commons-commons-collections4-4.1.jar [19] - lib/org.apache.commons-commons-lang3-3.6.jar [20] - lib/org.apache.zookeeper-zookeeper-3.8.1.jar [21] @@ -335,7 +334,6 @@ Apache Software License, Version 2. [15] Source available at https://github.com/eclipse/vert.x/tree/4.3.2 [16] Source available at https://github.com/vert-x3/vertx-web/tree/4.3.2 [17] Source available at https://github.com/apache/logging-log4j2/tree/rel/2.18.0 -[18] Source available at https://github.com/java-native-access/jna/tree/5.12.1 [19] Source available at https://github.com/apache/commons-collections/tree/collections-4.1 [20] Source available at https://github.com/apache/commons-lang/tree/LANG_3_6 [21] Source available at https://github.com/apache/zookeeper/tree/release-3.8.0 diff --git a/bookkeeper-server/pom.xml b/bookkeeper-server/pom.xml index 71782825ed..82dbadec47 100644 --- a/bookkeeper-server/pom.xml +++ b/bookkeeper-server/pom.xml @@ -125,10 +125,6 @@ <groupId>com.beust</groupId> <artifactId>jcommander</artifactId> </dependency> - <dependency> - <groupId>net.java.dev.jna</groupId> - <artifactId>jna</artifactId> - </dependency> <dependency> <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java index 9be4eee3b8..5c5a02252d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalChannel.java @@ -30,7 +30,7 @@ import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.util.Arrays; import org.apache.bookkeeper.conf.ServerConfiguration; -import org.apache.bookkeeper.util.NativeIO; +import org.apache.bookkeeper.util.PageCacheUtil; import org.apache.bookkeeper.util.ZeroBuffer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -241,7 +241,7 @@ class JournalChannel implements Closeable { } } if (fRemoveFromPageCache) { - this.fd = NativeIO.getSysFileDescriptor(channel.getFD()); + this.fd = PageCacheUtil.getSysFileDescriptor(channel.getFD()); } else { this.fd = -1; } @@ -323,7 +323,7 @@ class JournalChannel implements Closeable { if (fRemoveFromPageCache) { long newDropPos = newForceWritePosition - cacheDropLagBytes; if (lastDropPosition < newDropPos) { - NativeIO.bestEffortRemoveFromPageCache(fd, lastDropPosition, newDropPos - lastDropPosition); + PageCacheUtil.bestEffortRemoveFromPageCache(fd, lastDropPosition, newDropPos - lastDropPosition); } this.lastDropPosition = newDropPos; } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/NativeIO.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/PageCacheUtil.java similarity index 58% rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/util/NativeIO.java rename to bookkeeper-server/src/main/java/org/apache/bookkeeper/util/PageCacheUtil.java index d509053aa7..5dc1a5d8cc 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/NativeIO.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/PageCacheUtil.java @@ -18,41 +18,37 @@ package org.apache.bookkeeper.util; -import com.sun.jna.LastErrorException; -import com.sun.jna.Native; import java.io.FileDescriptor; import java.lang.reflect.Field; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import lombok.experimental.UtilityClass; +import lombok.extern.slf4j.Slf4j; +import org.apache.bookkeeper.common.util.nativeio.NativeIO; +import org.apache.bookkeeper.common.util.nativeio.NativeIOImpl; /** * Native I/O operations. */ -public final class NativeIO { - private static final Logger LOG = LoggerFactory.getLogger(NativeIO.class); +@UtilityClass +@Slf4j +public final class PageCacheUtil { private static final int POSIX_FADV_DONTNEED = 4; /* fadvise.h */ - private static boolean initialized = false; private static boolean fadvisePossible = true; + private static final NativeIO NATIVE_IO; + static { + NativeIO nativeIO = null; try { - Native.register("c"); - initialized = true; - } catch (NoClassDefFoundError e) { - LOG.info("JNA not found. Native methods will be disabled."); - } catch (UnsatisfiedLinkError e) { - LOG.info("Unable to link C library. Native methods will be disabled."); - } catch (NoSuchMethodError e) { - LOG.warn("Obsolete version of JNA present; unable to register C library"); + nativeIO = new NativeIOImpl(); + } catch (Exception e) { + log.warn("Unable to initialize NativeIO for posix_fdavise: {}", e.getMessage()); + fadvisePossible = false; } - } - - // fadvice - public static native int posix_fadvise(int fd, long offset, long len, int flag) throws LastErrorException; - private NativeIO() {} + NATIVE_IO = nativeIO; + } private static Field getFieldByReflection(Class cls, String fieldName) { Field field = null; @@ -63,7 +59,7 @@ public final class NativeIO { } catch (Exception e) { // We don't really expect this so throw an assertion to // catch this during development - LOG.warn("Unable to read {} field from {}", fieldName, cls.getName()); + log.warn("Unable to read {} field from {}", fieldName, cls.getName()); assert false; } @@ -79,14 +75,14 @@ public final class NativeIO { try { return field.getInt(descriptor); } catch (Exception e) { - LOG.warn("Unable to read fd field from java.io.FileDescriptor"); + log.warn("Unable to read fd field from java.io.FileDescriptor"); } return -1; } /** - * Remove pages from the file system page cache when they wont + * Remove pages from the file system page cache when they won't * be accessed again. * * @param fd The file descriptor of the source file. @@ -94,26 +90,14 @@ public final class NativeIO { * @param len The length to be flushed. */ public static void bestEffortRemoveFromPageCache(int fd, long offset, long len) { - if (!initialized || !fadvisePossible || fd < 0) { + if (!fadvisePossible || fd < 0) { return; } try { - posix_fadvise(fd, offset, len, POSIX_FADV_DONTNEED); - } catch (UnsupportedOperationException uoe) { - LOG.warn("posix_fadvise is not supported : ", uoe); - fadvisePossible = false; - } catch (UnsatisfiedLinkError ule) { - // if JNA is unavailable just skipping Direct I/O - // instance of this class will act like normal RandomAccessFile - LOG.warn("Unsatisfied Link error: posix_fadvise failed on file descriptor {}, offset {} : ", - fd, offset, ule); - fadvisePossible = false; + NATIVE_IO.posix_fadvise(fd, offset, len, POSIX_FADV_DONTNEED); } catch (Exception e) { - // This is best effort anyway so lets just log that there was an - // exception and forget - LOG.warn("Unknown exception: posix_fadvise failed on file descriptor {}, offset {} : ", - fd, offset, e); + log.warn("Failed to perform posix_fadvise: {}", e.getMessage()); + fadvisePossible = false; } } - } diff --git a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIO.java b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIO.java index 36bedd050c..4a27544e80 100644 --- a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIO.java +++ b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIO.java @@ -47,6 +47,12 @@ public interface NativeIO { */ int fallocate(int fd, int mode, long offset, long len) throws NativeIOException; + /** + * posix_fadvise is a linux-only syscall, so callers must handle the possibility that it does + * not exist. + */ + int posix_fadvise(int fd, long offset, long len, int flag) throws NativeIOException; + int pwrite(int fd, long pointer, int count, long offset) throws NativeIOException; long posix_memalign(int alignment, int size) throws NativeIOException; diff --git a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOImpl.java b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOImpl.java index 4542db1fb0..0ae7fd3eae 100644 --- a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOImpl.java +++ b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOImpl.java @@ -40,6 +40,11 @@ public class NativeIOImpl implements NativeIO { return NativeIOJni.fallocate(fd, mode, offset, len); } + @Override + public int posix_fadvise(int fd, long offset, long len, int flag) throws NativeIOException { + return NativeIOJni.posix_fadvise(fd, offset, len, flag); + } + @Override public long lseek(int fd, long offset, int whence) throws NativeIOException { return NativeIOJni.lseek(fd, offset, whence); diff --git a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOJni.java b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOJni.java index 89a909c115..77d2e80b01 100644 --- a/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOJni.java +++ b/native-io/src/main/java/org/apache/bookkeeper/common/util/nativeio/NativeIOJni.java @@ -34,6 +34,8 @@ class NativeIOJni { */ static native int fallocate(int fd, int mode, long offset, long len) throws NativeIOException; + static native int posix_fadvise(int fd, long offset, long len, int flag) throws NativeIOException; + static native int pwrite(int fd, long pointer, int count, long offset) throws NativeIOException; static native long posix_memalign(int alignment, int size) throws NativeIOException; diff --git a/native-io/src/main/native-io-jni/cpp/native_io_jni.c b/native-io/src/main/native-io-jni/cpp/native_io_jni.c index d2044291bd..d3bc164bec 100644 --- a/native-io/src/main/native-io-jni/cpp/native_io_jni.c +++ b/native-io/src/main/native-io-jni/cpp/native_io_jni.c @@ -195,6 +195,27 @@ Java_org_apache_bookkeeper_common_util_nativeio_NativeIOJni_fallocate( #endif } +/* + * Class: org_apache_bookkeeper_common_util_nativeio_NativeIOJni + * Method: posix_fadvise + * Signature: (IJJI)I + */ +JNIEXPORT jint JNICALL +Java_org_apache_bookkeeper_common_util_nativeio_NativeIOJni_posix_1fadvise( + JNIEnv* env, jclass clazz, + jint fd, jlong offset, jlong len, jint flag) { +#ifdef __linux__ + int res = posix_fadvise(fd, offset, len, flag); + if (res == -1) { + throwExceptionWithErrno(env, "Failed to posix_fadvise"); + } + return res; +#else + throwException(env, "posix_fadvise is not available"); + return -1; +#endif +} + /* * Class: org_apache_bookkeeper_common_util_nativeio_NativeIOJni * Method: lseek diff --git a/pom.xml b/pom.xml index 39116ce976..a46ffaeaf8 100644 --- a/pom.xml +++ b/pom.xml @@ -144,7 +144,6 @@ <jmh.version>1.19</jmh.version> <jmock.version>2.8.2</jmock.version> <jsoup.version>1.14.3</jsoup.version> - <jna.version>5.12.1</jna.version> <junit.version>4.12</junit.version> <!-- required by zookeeper test utilities imported from ZooKeeper --> <junit5.version>5.8.2</junit5.version> @@ -333,13 +332,6 @@ <version>${lz4.version}</version> </dependency> - <!-- JNA --> - <dependency> - <groupId>net.java.dev.jna</groupId> - <artifactId>jna</artifactId> - <version>${jna.version}</version> - </dependency> - <!-- yaml dependencies --> <dependency> <groupId>org.yaml</groupId> diff --git a/shaded/distributedlog-core-shaded/pom.xml b/shaded/distributedlog-core-shaded/pom.xml index 0c9bc90aed..6ca0e6aeb7 100644 --- a/shaded/distributedlog-core-shaded/pom.xml +++ b/shaded/distributedlog-core-shaded/pom.xml @@ -78,7 +78,6 @@ <include>com.fasterxml.jackson.core:jackson-annotations</include> <include>com.google.guava:guava</include> <include>com.google.protobuf:protobuf-java</include> - <include>net.java.dev.jna:jna</include> <include>net.jpountz.lz4:lz4</include> <include>org.apache.bookkeeper:bookkeeper-common</include> <include>org.apache.bookkeeper:bookkeeper-common-allocator</include> @@ -160,11 +159,6 @@ <pattern>com.fasterxml.jackson</pattern> <shadedPattern>dlshade.com.fasterxml.jackson</shadedPattern> </relocation> - <!-- jna --> - <relocation> - <pattern>com.sun.jna</pattern> - <shadedPattern>dlshade.com.sun.jna</shadedPattern> - </relocation> <!-- guava & protobuf --> <relocation> <pattern>com.google</pattern>