You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by us...@apache.org on 2023/01/04 18:17:10 UTC
[lucene] branch main updated: Retire/deprecate per-instance MMapDirectory#setUseUnmap (#12066)
This is an automated email from the ASF dual-hosted git repository.
uschindler pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/lucene.git
The following commit(s) were added to refs/heads/main by this push:
new 7f483bd6180 Retire/deprecate per-instance MMapDirectory#setUseUnmap (#12066)
7f483bd6180 is described below
commit 7f483bd6180c5c88176cc7c212405680d67ae249
Author: Uwe Schindler <us...@apache.org>
AuthorDate: Wed Jan 4 19:17:03 2023 +0100
Retire/deprecate per-instance MMapDirectory#setUseUnmap (#12066)
---
lucene/CHANGES.txt | 5 +
.../org/apache/lucene/store/MMapDirectory.java | 111 ++++++++++++---------
.../store/MappedByteBufferIndexInputProvider.java | 55 +++++-----
.../store/MemorySegmentIndexInputProvider.java | 9 +-
.../org/apache/lucene/store/TestDirectory.java | 2 +-
.../tests/store/BaseChunkedDirectoryTestCase.java | 5 -
6 files changed, 107 insertions(+), 80 deletions(-)
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 8de313f43a0..48f7189e102 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -145,6 +145,11 @@ API Changes
provide best performance for filtering and sorting.
(Francisco Fernández Castaño, Adrien Grand)
+* GITHUB#12066: Retire/deprecate instance method MMapDirectory#setUseUnmap().
+ Like the new setting for MemorySegments, this feature is enabled by default and
+ can only be disabled globally by passing the following sysprop on Java command line:
+ "-Dorg.apache.lucene.store.MMapDirectory.enableUnmapHack=false" (Uwe Schindler)
+
New Features
---------------------
* GITHUB#11795: Add ByteWritesTrackingDirectoryWrapper to expose metrics for bytes merged, flushed, and overall
diff --git a/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java b/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java
index a1495fe33d4..834b6751265 100644
--- a/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java
+++ b/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java
@@ -21,12 +21,15 @@ import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.nio.channels.ClosedChannelException; // javadoc @link
import java.nio.file.Path;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.util.Locale;
import java.util.Optional;
import java.util.concurrent.Future;
import java.util.function.BiPredicate;
import java.util.logging.Logger;
import org.apache.lucene.util.Constants;
+import org.apache.lucene.util.SuppressForbidden;
/**
* File-based {@link Directory} implementation that uses mmap for reading, and {@link
@@ -44,10 +47,10 @@ import org.apache.lucene.util.Constants;
* performance of searches on a cold page cache at the expense of slowing down opening an index. See
* {@link #setPreload(BiPredicate)} for more details.
*
- * <p>Due to <a href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4724038">this bug</a> in
- * Sun's JRE, MMapDirectory's {@link IndexInput#close} is unable to close the underlying OS file
- * handle. Only when GC finally collects the underlying objects, which could be quite some time
- * later, will the file handle be closed.
+ * <p>Due to <a href="https://bugs.openjdk.org/browse/JDK-4724038">this bug</a> in OpenJDK,
+ * MMapDirectory's {@link IndexInput#close} is unable to close the underlying OS file handle. Only
+ * when GC finally collects the underlying objects, which could be quite some time later, will the
+ * file handle be closed.
*
* <p>This will consume additional transient disk usage: on Windows, attempts to delete or overwrite
* the files will result in an exception; on other platforms, which typically have a "delete on
@@ -56,14 +59,26 @@ import org.apache.lucene.util.Constants;
* disk space, and you don't rely on overwriting files on Windows) but it's still an important
* limitation to be aware of.
*
- * <p>This class supplies the workaround mentioned in the bug report (see {@link #setUseUnmap}),
- * which may fail on non-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an
- * undocumented internal cleanup functionality. If {@link #UNMAP_SUPPORTED} is <code>true</code>,
- * the workaround will be automatically enabled (with no guarantees; if you discover any problems,
- * you can disable it).
+ * <p>This class supplies the workaround mentioned in the bug report, which may fail on
+ * non-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an undocumented
+ * internal cleanup functionality. If {@link #UNMAP_SUPPORTED} is <code>true</code>, the workaround
+ * will be automatically enabled (with no guarantees; if you discover any problems, you can disable
+ * it by using system property {@link #ENABLE_UNMAP_HACK_SYSPROP}).
+ *
+ * <p>For the hack to work correct, the following requirements need to be fulfilled: The used JVM
+ * must be at least Oracle Java / OpenJDK. In addition, the following permissions need to be granted
+ * to {@code lucene-core.jar} in your <a
+ * href="http://docs.oracle.com/javase/8/docs/technotes/guides/security/PolicyFiles.html">policy
+ * file</a>:
+ *
+ * <ul>
+ * <li>{@code permission java.lang.reflect.ReflectPermission "suppressAccessChecks";}
+ * <li>{@code permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";}
+ * </ul>
*
* <p>On exactly <b>Java 19</b> this class will use the modern {@code MemorySegment} API which
- * allows to safely unmap.
+ * allows to safely unmap (if you discover any problems with this preview API, you can disable it by
+ * using system property {@link #ENABLE_MEMORY_SEGMENTS_SYSPROP}).
*
* <p><b>NOTE:</b> Accessing this class either directly or indirectly from a thread while it's
* interrupted can close the underlying channel immediately if at the same time the thread is
@@ -81,6 +96,8 @@ import org.apache.lucene.util.Constants;
*/
public class MMapDirectory extends FSDirectory {
+ private static final Logger LOG = Logger.getLogger(MMapDirectory.class.getName());
+
/**
* Argument for {@link #setPreload(BiPredicate)} that configures all files to be preloaded upon
* opening them.
@@ -100,7 +117,6 @@ public class MMapDirectory extends FSDirectory {
public static final BiPredicate<String, IOContext> BASED_ON_LOAD_IO_CONTEXT =
(filename, context) -> context.load;
- private boolean useUnmapHack = UNMAP_SUPPORTED;
private BiPredicate<String, IOContext> preload = NO_FILES;
/**
@@ -114,6 +130,17 @@ public class MMapDirectory extends FSDirectory {
*/
public static final long DEFAULT_MAX_CHUNK_SIZE;
+ /**
+ * This sysprop allows to control the workaround/hack for unmapping the buffers from address space
+ * after closing {@link IndexInput}. By default it is enabled; set to {@code false} to disable the
+ * unmap hack globally. On command line pass {@code
+ * -Dorg.apache.lucene.store.MMapDirectory.enableUnmapHack=false} to disable.
+ *
+ * @lucene.internal
+ */
+ public static final String ENABLE_UNMAP_HACK_SYSPROP =
+ "org.apache.lucene.store.MMapDirectory.enableUnmapHack";
+
/**
* This sysprop allows to control if {@code MemorySegment} API should be used on supported Java
* versions. By default it is enabled; set to {@code false} to use legacy {@code ByteBuffer}
@@ -195,47 +222,29 @@ public class MMapDirectory extends FSDirectory {
}
/**
- * This method enables the workaround for unmapping the buffers from address space after closing
- * {@link IndexInput}, that is mentioned in the bug report. This hack may fail on
- * non-Oracle/OpenJDK JVMs. It forcefully unmaps the buffer on close by using an undocumented
- * internal cleanup functionality.
- *
- * <p>On exactly Java 19 this class will use the modern {@code MemorySegment} API which allows to
- * safely unmap. <em>The following warnings no longer apply in that case!</em>
+ * This method is retired, see deprecation notice!
*
- * <p><b>NOTE:</b> Enabling this is completely unsupported by Java and may lead to JVM crashes if
- * <code>IndexInput</code> is closed while another thread is still accessing it (SIGSEGV).
- *
- * <p>To enable the hack, the following requirements need to be fulfilled: The used JVM must be
- * Oracle Java / OpenJDK 8 <em>(preliminary support for Java 9 EA build 150+ was added with Lucene
- * 6.4)</em>. In addition, the following permissions need to be granted to {@code lucene-core.jar}
- * in your <a
- * href="http://docs.oracle.com/javase/8/docs/technotes/guides/security/PolicyFiles.html">policy
- * file</a>:
- *
- * <ul>
- * <li>{@code permission java.lang.reflect.ReflectPermission "suppressAccessChecks";}
- * <li>{@code permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";}
- * </ul>
- *
- * @throws IllegalArgumentException if {@link #UNMAP_SUPPORTED} is <code>false</code> and the
- * workaround cannot be enabled. The exception message also contains an explanation why the
- * hack cannot be enabled (e.g., missing permissions).
+ * @throws UnsupportedOperationException as setting cannot be changed
+ * @deprecated Please use new system property {@link #ENABLE_UNMAP_HACK_SYSPROP} instead
*/
+ @Deprecated(forRemoval = true)
public void setUseUnmap(final boolean useUnmapHack) {
- if (useUnmapHack && !UNMAP_SUPPORTED) {
- throw new IllegalArgumentException(UNMAP_NOT_SUPPORTED_REASON);
+ if (useUnmapHack != UNMAP_SUPPORTED) {
+ throw new UnsupportedOperationException(
+ "It is no longer possible configure unmap hack for directory instances. Please use the global system property: "
+ + ENABLE_UNMAP_HACK_SYSPROP);
}
- this.useUnmapHack = useUnmapHack;
}
/**
* Returns <code>true</code>, if the unmap workaround is enabled.
*
* @see #setUseUnmap
+ * @deprecated use {@link #UNMAP_SUPPORTED}
*/
+ @Deprecated
public boolean getUseUnmap() {
- return useUnmapHack;
+ return UNMAP_SUPPORTED;
}
/**
@@ -289,8 +298,7 @@ public class MMapDirectory extends FSDirectory {
ensureOpen();
ensureCanRead(name);
Path path = directory.resolve(name);
- return PROVIDER.openInput(
- path, context, chunkSizePower, preload.test(name, context), useUnmapHack);
+ return PROVIDER.openInput(path, context, chunkSizePower, preload.test(name, context));
}
// visible for tests:
@@ -306,8 +314,7 @@ public class MMapDirectory extends FSDirectory {
public static final String UNMAP_NOT_SUPPORTED_REASON;
static interface MMapIndexInputProvider {
- IndexInput openInput(
- Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+ IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
throws IOException;
long getDefaultMaxChunkSize();
@@ -359,6 +366,13 @@ public class MMapDirectory extends FSDirectory {
}
}
+ // Extracted to a method to be able to apply the SuppressForbidden annotation
+ @SuppressWarnings("removal")
+ @SuppressForbidden(reason = "security manager")
+ private static <T> T doPrivileged(PrivilegedAction<T> action) {
+ return AccessController.doPrivileged(action);
+ }
+
private static boolean checkMemorySegmentsSysprop() {
try {
return Optional.ofNullable(System.getProperty(ENABLE_MEMORY_SEGMENTS_SYSPROP))
@@ -367,6 +381,10 @@ public class MMapDirectory extends FSDirectory {
} catch (
@SuppressWarnings("unused")
SecurityException ignored) {
+ LOG.warning(
+ "Cannot read sysprop "
+ + ENABLE_MEMORY_SEGMENTS_SYSPROP
+ + ", so MemorySegments will be enabled by default, if possible.");
return true;
}
}
@@ -398,15 +416,14 @@ public class MMapDirectory extends FSDirectory {
"MemorySegmentIndexInputProvider is missing in Lucene JAR file", cnfe);
}
} else if (runtimeVersion >= 20) {
- var log = Logger.getLogger(lookup.lookupClass().getName());
- log.warning(
+ LOG.warning(
"You are running with Java 20 or later. To make full use of MMapDirectory, please update Apache Lucene.");
}
return new MappedByteBufferIndexInputProvider();
}
static {
- PROVIDER = lookupProvider();
+ PROVIDER = doPrivileged(MMapDirectory::lookupProvider);
DEFAULT_MAX_CHUNK_SIZE = PROVIDER.getDefaultMaxChunkSize();
UNMAP_SUPPORTED = PROVIDER.isUnmapSupported();
UNMAP_NOT_SUPPORTED_REASON = PROVIDER.getUnmapNotSupportedReason();
diff --git a/lucene/core/src/java/org/apache/lucene/store/MappedByteBufferIndexInputProvider.java b/lucene/core/src/java/org/apache/lucene/store/MappedByteBufferIndexInputProvider.java
index 798c924c02c..9f631ada5c5 100644
--- a/lucene/core/src/java/org/apache/lucene/store/MappedByteBufferIndexInputProvider.java
+++ b/lucene/core/src/java/org/apache/lucene/store/MappedByteBufferIndexInputProvider.java
@@ -30,9 +30,8 @@ import java.nio.channels.FileChannel;
import java.nio.channels.FileChannel.MapMode;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
import java.util.Objects;
+import java.util.Optional;
import java.util.logging.Logger;
import org.apache.lucene.store.ByteBufferGuard.BufferCleaner;
import org.apache.lucene.util.Constants;
@@ -40,13 +39,16 @@ import org.apache.lucene.util.SuppressForbidden;
final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapIndexInputProvider {
+ private static final Logger LOG =
+ Logger.getLogger(MappedByteBufferIndexInputProvider.class.getName());
+
private final BufferCleaner cleaner;
private final boolean unmapSupported;
private final String unmapNotSupportedReason;
public MappedByteBufferIndexInputProvider() {
- final Object hack = doPrivileged(MappedByteBufferIndexInputProvider::unmapHackImpl);
+ final Object hack = unmapHackImpl();
if (hack instanceof BufferCleaner) {
cleaner = (BufferCleaner) hack;
unmapSupported = true;
@@ -55,13 +57,12 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
cleaner = null;
unmapSupported = false;
unmapNotSupportedReason = hack.toString();
- Logger.getLogger(getClass().getName()).warning(unmapNotSupportedReason);
+ LOG.warning(unmapNotSupportedReason);
}
}
@Override
- public IndexInput openInput(
- Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+ public IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
throws IOException {
if (chunkSizePower > 30) {
throw new IllegalArgumentException(
@@ -77,7 +78,7 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
map(resourceDescription, fc, chunkSizePower, preload, fileSize),
fileSize,
chunkSizePower,
- new ByteBufferGuard(resourceDescription, useUnmapHack ? cleaner : null));
+ new ByteBufferGuard(resourceDescription, cleaner));
}
}
@@ -132,15 +133,29 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
return buffers;
}
- // Extracted to a method to be able to apply the SuppressForbidden annotation
- @SuppressWarnings("removal")
- @SuppressForbidden(reason = "security manager")
- private static <T> T doPrivileged(PrivilegedAction<T> action) {
- return AccessController.doPrivileged(action);
+ private static boolean checkUnmapHackSysprop() {
+ try {
+ return Optional.ofNullable(System.getProperty(MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP))
+ .map(Boolean::valueOf)
+ .orElse(Boolean.TRUE);
+ } catch (
+ @SuppressWarnings("unused")
+ SecurityException ignored) {
+ LOG.warning(
+ "Cannot read sysprop "
+ + MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP
+ + ", so buffer unmap hack will be enabled by default, if possible.");
+ return true;
+ }
}
@SuppressForbidden(reason = "Needs access to sun.misc.Unsafe to enable hack")
private static Object unmapHackImpl() {
+ if (checkUnmapHackSysprop() == false) {
+ return "Unmapping was disabled by system property "
+ + MMapDirectory.ENABLE_UNMAP_HACK_SYSPROP
+ + "=false";
+ }
final Lookup lookup = lookup();
try {
// *** sun.misc.Unsafe unmapping (Java 9+) ***
@@ -180,18 +195,10 @@ final class MappedByteBufferIndexInputProvider implements MMapDirectory.MMapInde
if (!buffer.isDirect()) {
throw new IllegalArgumentException("unmapping only works with direct buffers");
}
- final Throwable error =
- doPrivileged(
- () -> {
- try {
- unmapper.invokeExact(buffer);
- return null;
- } catch (Throwable t) {
- return t;
- }
- });
- if (error != null) {
- throw new IOException("Unable to unmap the mapped buffer: " + resourceDescription, error);
+ try {
+ unmapper.invokeExact(buffer);
+ } catch (Throwable t) {
+ throw new IOException("Unable to unmap the mapped buffer: " + resourceDescription, t);
}
};
}
diff --git a/lucene/core/src/java19/org/apache/lucene/store/MemorySegmentIndexInputProvider.java b/lucene/core/src/java19/org/apache/lucene/store/MemorySegmentIndexInputProvider.java
index f8ea1625cee..deaca72ae41 100644
--- a/lucene/core/src/java19/org/apache/lucene/store/MemorySegmentIndexInputProvider.java
+++ b/lucene/core/src/java19/org/apache/lucene/store/MemorySegmentIndexInputProvider.java
@@ -31,12 +31,15 @@ import org.apache.lucene.util.Unwrappable;
final class MemorySegmentIndexInputProvider implements MMapDirectory.MMapIndexInputProvider {
public MemorySegmentIndexInputProvider() {
- Logger.getLogger(getClass().getName()).info("Using MemorySegmentIndexInput with Java 19");
+ var log = Logger.getLogger(getClass().getName());
+ log.info(
+ "Using MemorySegmentIndexInput with Java 19; to disable start with -D"
+ + MMapDirectory.ENABLE_MEMORY_SEGMENTS_SYSPROP
+ + "=false");
}
@Override
- public IndexInput openInput(
- Path path, IOContext context, int chunkSizePower, boolean preload, boolean useUnmapHack)
+ public IndexInput openInput(Path path, IOContext context, int chunkSizePower, boolean preload)
throws IOException {
final String resourceDescription = "MemorySegmentIndexInput(path=\"" + path.toString() + "\")";
diff --git a/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java
index eef3d70340d..ec8447841cb 100644
--- a/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java
+++ b/lucene/core/src/test/org/apache/lucene/store/TestDirectory.java
@@ -65,7 +65,7 @@ public class TestDirectory extends LuceneTestCase {
assertEquals(1 + largeBuffer.length, d2.fileLength(fname));
// don't do read tests if unmapping is not supported!
- if (d2 instanceof MMapDirectory && !((MMapDirectory) d2).getUseUnmap()) continue;
+ if (d2 instanceof MMapDirectory && !MMapDirectory.UNMAP_SUPPORTED) continue;
IndexInput input = d2.openInput(fname, newIOContext(random()));
assertEquals((byte) i, input.readByte());
diff --git a/lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseChunkedDirectoryTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseChunkedDirectoryTestCase.java
index 19536526887..29442df503d 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseChunkedDirectoryTestCase.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/tests/store/BaseChunkedDirectoryTestCase.java
@@ -28,7 +28,6 @@ import org.apache.lucene.store.Directory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
-import org.apache.lucene.store.MMapDirectory;
import org.apache.lucene.tests.analysis.MockAnalyzer;
import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.tests.util.TestUtil;
@@ -267,10 +266,6 @@ public abstract class BaseChunkedDirectoryTestCase extends BaseDirectoryTestCase
private void assertChunking(Random random, int chunkSize) throws Exception {
Path path = createTempDir("mmap" + chunkSize);
Directory chunkedDir = getDirectory(path, chunkSize);
- // we will map a lot, try to turn on the unmap hack
- if (chunkedDir instanceof MMapDirectory && MMapDirectory.UNMAP_SUPPORTED) {
- ((MMapDirectory) chunkedDir).setUseUnmap(true);
- }
MockDirectoryWrapper dir = new MockDirectoryWrapper(random, chunkedDir);
RandomIndexWriter writer =
new RandomIndexWriter(