Skip to content

Conversation

@danielmitterdorfer
Copy link
Member

With this commit we rename node.store.allow_mmapfs to
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,
node.store.allow_mmapfs 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 #36668

With this commit we rename `node.store.allow_mmapfs` to
`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,
`node.store.allow_mmapfs` 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 elastic#36668
@danielmitterdorfer danielmitterdorfer added >enhancement v7.0.0 :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. labels Jan 2, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

[float]
==== Disabling memory-mapping

* The deprecated `node.store.allow_mmapfs` setting has been removed in favour of
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will create a separate PR on 6.x which introduces node.store.allow_mmapfs as a deprecated setting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering that saying "renamed to" rather than "removed in favour of" might be clearer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I changed it now to

The setting node.store.allow_mmapfs has been renamed to node.store.allow_mmap.

[float]
==== Disabling memory-mapping

* The deprecated `node.store.allow_mmapfs` setting has been removed in favour of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering that saying "renamed to" rather than "removed in favour of" might be clearer?

@danielmitterdorfer
Copy link
Member Author

@elasticmachine test this please

@danielmitterdorfer danielmitterdorfer merged commit 75f3443 into elastic:master Jan 3, 2019
@danielmitterdorfer danielmitterdorfer deleted the rename-allow-mmapfs branch January 3, 2019 06:31
danielmitterdorfer added a commit that referenced this pull request Jan 4, 2019
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
kovrus added a commit to crate/crate that referenced this pull request Sep 6, 2019
kovrus added a commit to crate/crate that referenced this pull request Sep 9, 2019
kovrus added a commit to crate/crate that referenced this pull request Sep 9, 2019
mergify bot pushed a commit to crate/crate that referenced this pull request Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants