Bugfix: Disable eager loading BitSetFilterCache on Indexing Nodes #105791
Bugfix: Disable eager loading BitSetFilterCache on Indexing Nodes #105791JVerwolf merged 9 commits intoelastic:mainfrom
Conversation
elasticsearchmachine
added
v8.14.0
needs:triage
JVerwolf
added
:Search/Search
|
Pinging @elastic/es-search (Team:Search) |
| boolean loadFiltersEagerlySetting = settings.getValue(INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING); | ||
| boolean hasIndexRole = DiscoveryNode.hasRole(settings.getNodeSettings(), DiscoveryNodeRole.INDEX_ROLE); | ||
| boolean isStateless = DiscoveryNode.isStateless(settings.getNodeSettings()); | ||
| if (isStateless && hasIndexRole) { |
There was a problem hiding this comment.
I added a check for stateless here. Technically, the INDEX_FAST_REFRESH_SETTING can only be set to true in stateless so this check is redundant. However, I'd rather explicitly check this requirement here so that the safety of this is not dependent on validation logic elsewhere, should the code elsewhere ever change.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I spent a bunch of time working on an integration test for this to check cache warming behaviour (see this diff: https://github.com/JVerwolf/elasticsearch/compare/bugfix/BitSetFilterCache...JVerwolf:elasticsearch:bugfix/BitSetFilterCache-IT-save2?expand=1#).
However, I ran into issues updating system indices and decided it wasn't worth the effort.
|
Hi @JVerwolf, I've created a changelog YAML for you. |
original-brownbear
left a comment
original-brownbear
left a comment
There was a problem hiding this comment.
LGTM if we move the one mostly redundant setting check into the condition :)
| boolean loadFiltersEagerlySetting = settings.getValue(INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING); | ||
| boolean hasIndexRole = DiscoveryNode.hasRole(settings.getNodeSettings(), DiscoveryNodeRole.INDEX_ROLE); | ||
| boolean isStateless = DiscoveryNode.isStateless(settings.getNodeSettings()); | ||
| if (isStateless && hasIndexRole) { |
There was a problem hiding this comment.
Let's evaluate hasIndexRole after isStateless then I'd say :) Just so we don't pay for both checks during normal operation (not that it matters too much but still).
…csearch into bugfix/BitSetFilterCache
|
@elasticmachine update branch |
|
Hi @JVerwolf, I've updated the changelog YAML for you. |
4 participants
This PR disables eager loading of the BitSetFilterCache for stateless indexing nodes.