New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install a Graylog index template instead of set mappings on index creation #1628

Merged
merged 4 commits into from Dec 15, 2015

Conversation

Projects
None yet
2 participants
@bernd
Member

bernd commented Dec 11, 2015

In commit edf0d8b we changed the index creation process from "create index" + "install index mapping" to one operation to avoid race conditions.

This broke setups where users had installed custom Elasticsearch index templates because the user templates where applied after our mapping was installed into the index. Before edf0d8b the user templates have been applied before we installed our mapping because the index creation was split into two steps.

We now install a "graylog-internal" index template into Elasticsearch with a "order" or -99 to make sure user templates are able to override our settings.

Fixes #1624

@bernd bernd added this to the 1.3.1 milestone Dec 11, 2015

bernd added some commits Dec 11, 2015

Install a Graylog index template instead of set mappings on index cre…
…ation

In commit edf0d8b we changed the index creation process from "create
index" + "install index mapping" to one operation to avoid race
conditions.

This broke setups where users had installed custom Elasticsearch index
templates because the user templates where applied after our mapping was
installed into the index. Before edf0d8b the user templates have been
applied before we installed our mapping because the index creation was
split into two steps.

We now install a "graylog-internal" index template into Elasticsearch
with a "order" or -99 to make sure user templates are able to override
our settings.

Fixes #1624
Make tests work again after switching to an index template
- Pass the ES config object intot the IndexCreatingLoadStrategyFactory
  to make sure the index is created with the correct settings.
- Adjust index name in all fixtures.

@bernd bernd force-pushed the issue-1624 branch from a223895 to ea9c38f Dec 14, 2015

@joschi joschi self-assigned this Dec 15, 2015

public boolean createIndexTemplate() {
final Map<String, Object> template = indexMapping.messageTemplate(allIndicesAlias(), configuration.getAnalyzer());
final PutIndexTemplateRequest itr = c.admin().indices().preparePutTemplate(GRAYLOG_INTERNAL_TEMPLATE_NAME)
.setOrder(-99) // Make sure templates with "order: 0" are applied after our template!

This comment has been minimized.

@joschi

joschi Dec 15, 2015

Contributor

Why -99 and not -1 or Integer.MIN_VALUE?

This comment has been minimized.

@bernd

bernd Dec 15, 2015

Member

I chose -99 because I didn't want to use -1 because we might need some space in the future. 😉

I will change it to Integer.MIN_VALUE.

@@ -99,6 +100,8 @@
private static final Logger LOG = LoggerFactory.getLogger(Indices.class);
private static final String GRAYLOG_INTERNAL_TEMPLATE_NAME = "graylog-internal";

This comment has been minimized.

@joschi

joschi Dec 15, 2015

Contributor

Why "internal"? Is there another Graylog index template?

This comment has been minimized.

@bernd

bernd Dec 15, 2015

Member

I used that name to make it clear that this is our Graylog internal mapping and because users might have a template named "graylog" already.

This comment has been minimized.

@joschi

joschi Dec 15, 2015

Contributor

Fair enough.

@@ -211,6 +214,26 @@ public String aliasTarget(String alias) {
return response.getAliases().isEmpty() ? null : response.getAliases().keysIt().next();
}
public boolean createIndexTemplate() {
final Map<String, Object> template = indexMapping.messageTemplate(allIndicesAlias(), configuration.getAnalyzer());
final PutIndexTemplateRequest itr = c.admin().indices().preparePutTemplate(GRAYLOG_INTERNAL_TEMPLATE_NAME)

This comment has been minimized.

@joschi

joschi Dec 15, 2015

Contributor

Could this cause problems if the template already exists and/or has been modified?

This comment has been minimized.

@bernd

bernd Dec 15, 2015

Member

It will just overwrite any changes with our configured values, which is what we want I guess.

bernd added some commits Dec 15, 2015

Install the index template in Indices#create()
This is needed to ensure that the template is created under all
circumstances. This still leaves a race condition where the template is
not propagated throughout the cluster when a new index gets created. Not
much we can do about this right now.

@bernd bernd force-pushed the issue-1624 branch from 16bc453 to d5846fa Dec 15, 2015

@bernd

This comment has been minimized.

Member

bernd commented Dec 15, 2015

Updated after some discussions.

@joschi

This comment has been minimized.

Contributor

joschi commented Dec 15, 2015

LGTM. 👍

joschi added a commit that referenced this pull request Dec 15, 2015

Merge pull request #1628 from Graylog2/issue-1624
Install a Graylog index template instead of set mappings on index creation

@joschi joschi merged commit 14db43e into 1.3 Dec 15, 2015

1 of 3 checks passed

ci Jenkins build graylog2-server-integration-pr 454 has failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@joschi joschi deleted the issue-1624 branch Dec 15, 2015

joschi added a commit that referenced this pull request Dec 15, 2015

Merge pull request #1628 from Graylog2/issue-1624
Install a Graylog index template instead of set mappings on index creation
(cherry picked from commit 14db43e)
@joschi

This comment has been minimized.

Contributor

joschi commented Dec 15, 2015

Merged into master in be54753.

joschi added a commit that referenced this pull request Dec 17, 2015

joschi added a commit that referenced this pull request Dec 17, 2015

Make Elasticsearch index template name configurable
Refs #1624, refs #1628
(cherry picked from commit a826c05)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment