Skip to content

Commit

Permalink
LUCENE-9576: nuke SSD detection, modernize CMS defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmuir
rmuir committed Oct 17, 2020
1 parent f7be9e8 commit 85b58c2
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 622 deletions.
2 changes: 0 additions & 2 deletions gradle/testing/randomization/policies/solr-tests.policy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ grant {
permission java.lang.RuntimePermission "getStackTrace";
// needed for mock filesystems in tests
permission java.lang.RuntimePermission "fileSystemProvider";
// needed for test of IOUtils.spins (maybe it can be avoided)
permission java.lang.RuntimePermission "getFileStoreAttributes";
// analyzers/uima: needed by lucene expressions' JavascriptCompiler
permission java.lang.RuntimePermission "createClassLoader";
// needed to test unmap hack on platforms that support it
Expand Down
3 changes: 0 additions & 3 deletions gradle/testing/randomization/policies/tests.policy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ grant {
permission java.lang.RuntimePermission "getStackTrace";
// needed for mock filesystems in tests
permission java.lang.RuntimePermission "fileSystemProvider";
// needed for test of IOUtils.spins (maybe it can be avoided)
permission java.lang.RuntimePermission "getFileStoreAttributes";
// analyzers/uima: needed by lucene expressions' JavascriptCompiler
permission java.lang.RuntimePermission "createClassLoader";
// needed to test unmap hack on platforms that support it
Expand All @@ -77,7 +75,6 @@ grant {

// CMS randomization
permission java.util.PropertyPermission "lucene.cms.override_core_count", "write";
permission java.util.PropertyPermission "lucene.cms.override_spins", "write";

// used by nested tests? (e.g. TestLeaveFilesIfTestFails). TODO: look into this
permission java.util.PropertyPermission "tests.runnested", "write";
Expand Down
8 changes: 8 additions & 0 deletions lucene/CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,14 @@ API Changes

Improvements

* LUCENE-9576: Improve ConcurrentMergeScheduler settings by default, assuming modern I/O.
Previously Lucene was too conservative, jumping through hoops to detect if disks were SSD-backed.
In many common modern cases (VMs, RAID arrays, containers, encrypted mounts, non-Linux OS),
the pessimistic heuristics were wrong, resulting in slower indexing performance. Heuristics were
also complex and would trigger JDK issues even on unrelated mount points. Merge scheduler defaults
are now modernized and the heuristics removed. Users with spinning disks that want to maximize I/O
performance should tweak ConcurrentMergeScheduler. (Robert Muir)

* LUCENE-9463: Query match region retrieval component, passage scoring and formatting
for building custom highlighters. (Alan Woodward, Dawid Weiss)

Expand Down
45 changes: 10 additions & 35 deletions lucene/core/src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.lucene.store.RateLimitedIndexOutput;
import org.apache.lucene.store.RateLimiter;
import org.apache.lucene.util.CollectionUtil;
import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.InfoStream;
import org.apache.lucene.util.ThreadInterruptedException;

Expand All @@ -52,37 +51,25 @@
* incoming threads by pausing until one more merges
* complete.</p>
*
* <p>This class attempts to detect whether the index is
* on rotational storage (traditional hard drive) or not
* (e.g. solid-state disk) and changes the default max merge
* and thread count accordingly. This detection is currently
* Linux-only, and relies on the OS to put the right value
* into /sys/block/&lt;dev&gt;/block/rotational. For all
* other operating systems it currently assumes a rotational
* disk for backwards compatibility. To enable default
* settings for spinning or solid state disks for such
* operating systems, use {@link #setDefaultMaxMergesAndThreads(boolean)}.
* <p>This class sets defaults based on Java's view of the
* cpu count, and it assumes a solid state disk (or similar).
* If you have a spinning disk and want to maximize performance,
* use {@link #setDefaultMaxMergesAndThreads(boolean)}.
*/
public class ConcurrentMergeScheduler extends MergeScheduler {

/** Dynamic default for {@code maxThreadCount} and {@code maxMergeCount},
* used to detect whether the index is backed by an SSD or rotational disk and
* set {@code maxThreadCount} accordingly. If it's an SSD,
* {@code maxThreadCount} is set to {@code max(1, min(4, cpuCoreCount/2))},
* otherwise 1. Note that detection only currently works on
* Linux; other platforms will assume the index is not on an SSD. */
* based on CPU core count.
* {@code maxThreadCount} is set to {@code max(1, min(4, cpuCoreCount/2))}.
* {@code maxMergeCount} is set to {@code maxThreadCount + 5}.
*/
public static final int AUTO_DETECT_MERGES_AND_THREADS = -1;

/** Used for testing.
*
* @lucene.internal */
public static final String DEFAULT_CPU_CORE_COUNT_PROPERTY = "lucene.cms.override_core_count";

/** Used for testing.
*
* @lucene.internal */
public static final String DEFAULT_SPINS_PROPERTY = "lucene.cms.override_spins";

/** List of currently active {@link MergeThread}s. */
protected final List<MergeThread> mergeThreads = new ArrayList<>();

Expand Down Expand Up @@ -410,21 +397,9 @@ protected synchronized void updateMergeThreads() {

private synchronized void initDynamicDefaults(Directory directory) throws IOException {
if (maxThreadCount == AUTO_DETECT_MERGES_AND_THREADS) {
boolean spins = IOUtils.spins(directory);

// Let tests override this to help reproducing a failure on a machine that has a different
// core count than the one where the test originally failed:
try {
String value = System.getProperty(DEFAULT_SPINS_PROPERTY);
if (value != null) {
spins = Boolean.parseBoolean(value);
}
} catch (Exception ignored) {
// that's fine we might hit a SecurityException etc. here just continue
}
setDefaultMaxMergesAndThreads(spins);
setDefaultMaxMergesAndThreads(false);
if (verbose()) {
message("initDynamicDefaults spins=" + spins + " maxThreadCount=" + maxThreadCount + " maxMergeCount=" + maxMergeCount);
message("initDynamicDefaults maxThreadCount=" + maxThreadCount + " maxMergeCount=" + maxMergeCount);
}
}
}
Expand Down
147 changes: 0 additions & 147 deletions lucene/core/src/java/org/apache/lucene/util/IOUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
import java.nio.charset.CharsetDecoder;
import java.nio.charset.CodingErrorAction;
import java.nio.charset.StandardCharsets;
import java.nio.file.DirectoryStream;
import java.nio.file.FileStore;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
Expand All @@ -42,11 +40,7 @@
import java.util.Map;
import java.util.Objects;

import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FSDirectory;
import org.apache.lucene.store.FileSwitchDirectory;
import org.apache.lucene.store.FilterDirectory;

/** This class emulates the new Java 7 "Try-With-Resources" statement.
* Remove once Lucene is on Java 7.
Expand Down Expand Up @@ -483,147 +477,6 @@ public static void fsync(Path fileToSync, boolean isDir) throws IOException {
}
}

/** If the dir is an {@link FSDirectory} or wraps one via possibly
* nested {@link FilterDirectory} or {@link FileSwitchDirectory},
* this returns {@link #spins(Path)} for the wrapped directory,
* else, true.
*
* @throws IOException if {@code path} does not exist.
*
* @lucene.internal */
public static boolean spins(Directory dir) throws IOException {
dir = FilterDirectory.unwrap(dir);
if (dir instanceof FileSwitchDirectory) {
FileSwitchDirectory fsd = (FileSwitchDirectory) dir;
// Spinning is contagious:
return spins(fsd.getPrimaryDir()) || spins(fsd.getSecondaryDir());
} else if (dir instanceof ByteBuffersDirectory) {
return false;
} else if (dir instanceof FSDirectory) {
return spins(((FSDirectory) dir).getDirectory());
} else {
return true;
}
}

/** Rough Linux-only heuristics to determine whether the provided
* {@code Path} is backed by spinning storage. For example, this
* returns false if the disk is a solid-state disk.
*
* @param path a location to check which must exist. the mount point will be determined from this location.
* @return false if the storage is non-rotational (e.g. an SSD), or true if it is spinning or could not be determined
* @throws IOException if {@code path} does not exist.
*
* @lucene.internal */
public static boolean spins(Path path) throws IOException {
// resolve symlinks (this will throw exception if the path does not exist)
path = path.toRealPath();

// Super cowboy approach, but seems to work!
if (!Constants.LINUX) {
return true; // no detection
}

try {
return spinsLinux(path);
} catch (Exception exc) {
// our crazy heuristics can easily trigger SecurityException, AIOOBE, etc ...
return true;
}
}

// following methods are package-private for testing ONLY

// note: requires a real or fake linux filesystem!
static boolean spinsLinux(Path path) throws IOException {
FileStore store = getFileStore(path);

// if fs type is tmpfs, it doesn't spin.
// this won't have a corresponding block device
if ("tmpfs".equals(store.type())) {
return false;
}

// get block device name
String devName = store.name();

// not a device (e.g. NFS server)
if (!devName.startsWith("/")) {
return true;
}

// resolve any symlinks to real block device (e.g. LVM)
// /dev/sda0 -> sda0
// /devices/XXX -> sda0
devName = path.getRoot().resolve(devName).toRealPath().getFileName().toString();

// now try to find the longest matching device folder in /sys/block
// (that starts with our dev name):
Path sysinfo = path.getRoot().resolve("sys").resolve("block");
Path devsysinfo = null;
int matchlen = 0;
try (DirectoryStream<Path> stream = Files.newDirectoryStream(sysinfo)) {
for (Path device : stream) {
String name = device.getFileName().toString();
if (name.length() > matchlen && devName.startsWith(name)) {
devsysinfo = device;
matchlen = name.length();
}
}
}

if (devsysinfo == null) {
return true; // give up
}

// read first byte from rotational, it's a 1 if it spins.
Path rotational = devsysinfo.resolve("queue").resolve("rotational");
try (InputStream stream = Files.newInputStream(rotational)) {
return stream.read() == '1';
}
}

// Files.getFileStore(Path) useless here!
// don't complain, just try it yourself
static FileStore getFileStore(Path path) throws IOException {
FileStore store = Files.getFileStore(path);
String mount = getMountPoint(store);

// find the "matching" FileStore from system list, it's the one we want, but only return
// that if it's unambiguous (only one matching):
FileStore sameMountPoint = null;
for (FileStore fs : path.getFileSystem().getFileStores()) {
if (mount.equals(getMountPoint(fs))) {
if (sameMountPoint == null) {
sameMountPoint = fs;
} else {
// more than one filesystem has the same mount point; something is wrong!
// fall back to crappy one we got from Files.getFileStore
return store;
}
}
}

if (sameMountPoint != null) {
// ok, we found only one, use it:
return sameMountPoint;
} else {
// fall back to crappy one we got from Files.getFileStore
return store;
}
}

// these are hacks that are not guaranteed, may change across JVM versions, etc.
static String getMountPoint(FileStore store) {
String desc = store.toString();
int index = desc.lastIndexOf(" (");
if (index != -1) {
return desc.substring(0, index);
} else {
return desc;
}
}

/**
* Returns the second throwable if the first is null otherwise adds the second as suppressed to the first
* and returns it.
Expand Down
Loading

0 comments on commit 85b58c2

Please sign in to comment.