Skip to content

Commit

Permalink
Deprecate allow mmapfs
Browse files Browse the repository at this point in the history
With this commit we deprecate the setting `node.store.allow_mmapfs` and
add a new setting `node.store.allow_mmap`. Previously this setting has
controlled whether `mmapfs` could be used as a store type. With the
introduction of `hybridfs` which also relies on memory-mapping the old
setting name is no longer appropriate as it also applies to
`hybridfs`and thus we rename it in order to convey that it is actually
used to allow memory-mapping but not a specific store type.

Relates #37067
Relates #37070
Relates #37095
  • Loading branch information
danielmitterdorfer authored Jan 4, 2019
1 parent f2747b0 commit 03b1ddd
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 24 deletions.
4 changes: 2 additions & 2 deletions docs/reference/index-modules/store.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ memory mapped. All other files are opened using Lucene `NIOFSDirectory`.
Similarly to `mmapfs` be sure you have allowed plenty of
<<vm-max-map-count,virtual address space>>.

[[allow-mmapfs]]
[[allow-mmap]]
You can restrict the use of the `mmapfs` and the related `hybridfs` store type
via the setting `node.store.allow_mmapfs`. This is a boolean setting indicating
via the setting `node.store.allow_mmap`. This is a boolean setting indicating
whether or not memory-mapping is allowed. The default is to allow it. This
setting is useful, for example, if you are in an environment where you can not
control the ability to create a lot of memory maps so you need disable the
Expand Down
11 changes: 11 additions & 0 deletions docs/reference/migration/migrate_6_7.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This section discusses the changes that you need to be aware of when migrating
your application to Elasticsearch 6.7.

* <<breaking_67_plugin_changes>>
* <<breaking_67_settings_changes>>

See also <<release-highlights>> and <<es-release-notes>>.

Expand All @@ -28,3 +29,13 @@ will result in an error. Additionally, there are two minor breaking changes here
- `plugin.mandatory` is no longer compatible with `ingest-geoip` nor
`ingest-user-agent`


[float]
[[breaking_67_settings_changes]]
=== Settings changes

[float]
==== Disabling memory-mapping

The setting `node.store.allow_mmapfs` has been deprecated. Use
`node.store.allow_mmap` instead.
6 changes: 3 additions & 3 deletions docs/reference/setup/bootstrap-checks.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ and is enforced on Linux only. To pass the maximum map count check, you
must configure `vm.max_map_count` via `sysctl` to be at least `262144`.

Alternatively, the maximum map count check is only needed if you are using
`mmapfs` as the <<index-modules-store,store type>> for your indices. If you
<<allow-mmapfs,do not allow>> the use of `mmapfs` then this bootstrap check will
not be enforced.
`mmapfs` or `hybridfs` as the <<index-modules-store,store type>> for your
indices. If you <<allow-mmap,do not allow>> the use of `mmap` then this
bootstrap check will not be enforced.

=== Client JVM check

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,8 @@ static class MaxMapCountCheck implements BootstrapCheck {

@Override
public BootstrapCheckResult check(final BootstrapContext context) {
// we only enforce the check if mmapfs is an allowed store type
if (IndexModule.NODE_STORE_ALLOW_MMAPFS.get(context.settings())) {
// we only enforce the check if a store is allowed to use mmap at all
if (IndexModule.NODE_STORE_ALLOW_MMAP.get(context.settings())) {
if (getMaxMapCount() != -1 && getMaxMapCount() < LIMIT) {
final String message = String.format(
Locale.ROOT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ public void apply(Settings value, Settings current, Settings previous) {
HierarchyCircuitBreakerService.ACCOUNTING_CIRCUIT_BREAKER_LIMIT_SETTING,
HierarchyCircuitBreakerService.ACCOUNTING_CIRCUIT_BREAKER_OVERHEAD_SETTING,
IndexModule.NODE_STORE_ALLOW_MMAPFS,
IndexModule.NODE_STORE_ALLOW_MMAP,
ClusterService.CLUSTER_SERVICE_SLOW_TASK_LOGGING_THRESHOLD_SETTING,
ClusterService.USER_DEFINED_META_DATA,
SearchService.DEFAULT_SEARCH_TIMEOUT_SETTING,
Expand Down
18 changes: 11 additions & 7 deletions server/src/main/java/org/elasticsearch/index/IndexModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@
*/
public final class IndexModule {

public static final Setting<Boolean> NODE_STORE_ALLOW_MMAPFS = Setting.boolSetting("node.store.allow_mmapfs", true, Property.NodeScope);
public static final Setting<Boolean> NODE_STORE_ALLOW_MMAPFS =
Setting.boolSetting("node.store.allow_mmapfs", true, Property.NodeScope, Property.Deprecated);

public static final Setting<Boolean> NODE_STORE_ALLOW_MMAP =
Setting.boolSetting("node.store.allow_mmap", NODE_STORE_ALLOW_MMAPFS, Property.NodeScope);

public static final Setting<String> INDEX_STORE_TYPE_SETTING =
new Setting<>("index.store.type", "", Function.identity(), Property.IndexScope, Property.NodeScope);
Expand Down Expand Up @@ -355,8 +359,8 @@ public interface IndexSearcherWrapperFactory {
IndexSearcherWrapper newWrapper(IndexService indexService);
}

public static Type defaultStoreType(final boolean allowMmapfs) {
if (allowMmapfs && Constants.JRE_IS_64BIT && MMapDirectory.UNMAP_SUPPORTED) {
public static Type defaultStoreType(final boolean allowMmap) {
if (allowMmap && Constants.JRE_IS_64BIT && MMapDirectory.UNMAP_SUPPORTED) {
return Type.MMAPFS;
} else if (Constants.WINDOWS) {
return Type.SIMPLEFS;
Expand Down Expand Up @@ -406,18 +410,18 @@ private static IndexStore getIndexStore(
final IndexSettings indexSettings, final Map<String, Function<IndexSettings, IndexStore>> indexStoreFactories) {
final String storeType = indexSettings.getValue(INDEX_STORE_TYPE_SETTING);
final Type type;
final Boolean allowMmapfs = NODE_STORE_ALLOW_MMAPFS.get(indexSettings.getNodeSettings());
final Boolean allowMmap = NODE_STORE_ALLOW_MMAP.get(indexSettings.getNodeSettings());
if (storeType.isEmpty() || Type.FS.getSettingsKey().equals(storeType)) {
type = defaultStoreType(allowMmapfs);
type = defaultStoreType(allowMmap);
} else {
if (isBuiltinType(storeType)) {
type = Type.fromSettingsKey(storeType);
} else {
type = null;
}
}
if (type != null && type == Type.MMAPFS && allowMmapfs == false) {
throw new IllegalArgumentException("store type [mmapfs] is not allowed");
if (allowMmap == false && (type == Type.MMAPFS || type == Type.HYBRIDFS)) {
throw new IllegalArgumentException("store type [" + storeType + "] is not allowed because mmap is disabled");
}
final IndexStore store;
if (storeType.isEmpty() || isBuiltinType(storeType)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory) throw
indexSettings.getSettings().get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey());
IndexModule.Type type;
if (IndexModule.Type.FS.match(storeType)) {
type = IndexModule.defaultStoreType(IndexModule.NODE_STORE_ALLOW_MMAPFS.get(indexSettings.getNodeSettings()));
type = IndexModule.defaultStoreType(IndexModule.NODE_STORE_ALLOW_MMAP.get(indexSettings.getNodeSettings()));
} else {
type = IndexModule.Type.fromSettingsKey(storeType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,21 @@ public void testMaxMapCountCheckBelowLimitAndMemoryMapAllowed() {
/*
* There are two ways that memory maps are allowed:
* - by default
* - mmapfs is explicitly allowed
* We want to test that if mmapfs is allowed then the max map count check is enforced.
* - mmap is explicitly allowed
* We want to test that if mmap is allowed then the max map count check is enforced.
*/
final List<Settings> settingsThatAllowMemoryMap = new ArrayList<>();
settingsThatAllowMemoryMap.add(Settings.EMPTY);
settingsThatAllowMemoryMap.add(Settings.builder().put("node.store.allow_mmapfs", true).build());
settingsThatAllowMemoryMap.add(Settings.builder().put("node.store.allow_mmap", true).build());

for (final Settings settingThatAllowsMemoryMap : settingsThatAllowMemoryMap) {
assertFailure(check.check(createTestContext(settingThatAllowsMemoryMap, MetaData.EMPTY_META_DATA)));
}
}

public void testMaxMapCountCheckNotEnforcedIfMemoryMapNotAllowed() {
// nothing should happen if current vm.max_map_count is under the limit but mmapfs is not allowed
final Settings settings = Settings.builder().put("node.store.allow_mmapfs", false).build();
// nothing should happen if current vm.max_map_count is under the limit but mmap is not allowed
final Settings settings = Settings.builder().put("node.store.allow_mmap", false).build();
final BootstrapContext context = createTestContext(settings, MetaData.EMPTY_META_DATA);
final BootstrapCheck.BootstrapCheckResult result = check.check(context);
assertTrue(result.isSuccess());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,19 +382,20 @@ public void testDisableQueryCacheHasPrecedenceOverForceQueryCache() throws IOExc
indexService.close("simon says", false);
}

public void testMmapfsStoreTypeNotAllowed() {
public void testMmapNotAllowed() {
String storeType = randomFrom(IndexModule.Type.HYBRIDFS.getSettingsKey(), IndexModule.Type.MMAPFS.getSettingsKey());
final Settings settings = Settings.builder()
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir())
.put("index.store.type", "mmapfs")
.put("index.store.type", storeType)
.build();
final Settings nodeSettings = Settings.builder()
.put(IndexModule.NODE_STORE_ALLOW_MMAPFS.getKey(), false)
.put(IndexModule.NODE_STORE_ALLOW_MMAP.getKey(), false)
.build();
final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(new Index("foo", "_na_"), settings, nodeSettings);
final IndexModule module =
new IndexModule(indexSettings, emptyAnalysisRegistry, new InternalEngineFactory(), Collections.emptyMap());
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> newIndexService(module));
assertThat(e, hasToString(containsString("store type [mmapfs] is not allowed")));
assertThat(e, hasToString(containsString("store type [" + storeType + "] is not allowed")));
}

class CustomQueryCache implements QueryCache {
Expand Down

0 comments on commit 03b1ddd

Please sign in to comment.