Ensure that .watcher-history-11* template is in installed prior to use #56734

jakelandis · 2020-05-14T01:28:58Z

WatcherIndexTemplateRegistry
as of #52962 requires all nodes
to be on 7.7.0 before it allows the version 11 index template to be installed.

While in a mixed cluster, nothing prevents Watcher from running on the new
host before the all of the nodes are on 7.7.0. This will result in the
.watcher-history-11* index without the proper mappings. Without the proper
mapping a single document (for a large watch) can exceed the default 1000 field
limit and cause error to show in the logs.

This commit ensures the same logic for writing to the index is applied as for
installing the template. In a mixed cluster, the 10 index template will continue
to be written. Only once all of nodes are on 7.7.0+ will the 11 index template
be installed and used.

Note - this PR targets 7.x and will be back ported to 7.7.next. This conditional
installation of the template is not present in master.

closes #56732

[WatcherIndexTemplateRegistry](https://github.com/elastic/elasticsearch/blob/7.7/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/WatcherIndexTemplateRegistry.java#L74) as of elastic#52962 requires all nodes to be on 7.7.0 before it allows the version 11 index template to be installed. While in a mixed cluster, nothing prevents Watcher from running on the new host before the all of the nodes are on 7.7.0. This will result in the .watcher-history-11* index without the proper mappings. Without the proper mapping a single document (for a large watch) can exceed the default 1000 field limit and cause error to show in the logs. This commit ensures the same logic for writing to the index is applied as for installing the template. In a mixed cluster, the `10` index template will continue to be written. Only once all of nodes are on 7.7.0+ will the `11` index template be installed and used. Note - this PR targets 7.x and will be back ported to 7.7.next. This conditional installation of the template is not present in master. closes elastic#56732

elasticmachine · 2020-05-14T01:29:00Z

Pinging @elastic/es-core-features (:Core/Features/Watcher)

jakelandis · 2020-05-14T01:29:49Z

@gwbrown - mind taking a look at this one? It touches some of the areas I know you are familiar with.

...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java

gwbrown

Good catch. Thanks for the fix and apologies for the breakage. Left a few relatively minor comments, but this looks like a good fix.

.../plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/history/HistoryStoreTests.java

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/history/HistoryStore.java

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java

gwbrown · 2020-05-14T22:21:11Z

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java

+        if (enabled && transportClient == false) {
+            validAutoCreateIndex(settings, logger, clusterService);
+        }


Just double checking: This doesn't result in different behavior, right? Just makes the failure occur (if it occurs) at a different time during node startup.

(also, my assumption is that this moved because at the time the constructor is called we don't have a cluster state yet, correct?)

correct, same behavior and moved to allow access the the cluster state.

jakelandis · 2020-05-15T14:37:36Z

@gwbrown thanks for the review, should be ready for another look - changes implemented on 123453c

EDIT: I noticed that the commit starts with 12345, what kind of prize do I win ?

jakelandis · 2020-05-15T15:06:16Z

@elasticmachine update branch

jakelandis · 2020-05-15T15:19:10Z

Test failures look related to attempting to read cluster state too early (specific to this PR)

jakelandis · 2020-05-15T15:28:58Z

Had to remove the conditional logic for the validAutoCreateIndex method. This relevant parts of this method only emits a warning if the auto_create_index setting is too restrictive to create daily indices for the next 6 months. For this case, it is fine (possibly even more correct) to always assume the latest version of the index name. Changes on 7a80068

gwbrown

LGTM, thanks! I agree that only checking the "newest" index name pattern is probably fine, it's very much a best-effort check anyway.

elastic#56734) WatcherIndexTemplateRegistry as of elastic#52962 requires all nodes to be on 7.7.0 before it allows the version 11 index template to be installed. While in a mixed cluster, nothing prevents Watcher from running on the new host before the all of the nodes are on 7.7.0. This will result in the .watcher-history-11* index without the proper mappings. Without the proper mapping a single document (for a large watch) can exceed the default 1000 field limit and cause error to show in the logs. This commit ensures the same logic for writing to the index is applied as for installing the template. In a mixed cluster, the `10` index template will continue to be written. Only once all of nodes are on 7.7.0+ will the `11` index template be installed and used. closes elastic#56732

… to use (#56734) (#56848) WatcherIndexTemplateRegistry as of #52962 requires all nodes to be on 7.7.0 before it allows the version 11 index template to be installed. While in a mixed cluster, nothing prevents Watcher from running on the new host before the all of the nodes are on 7.7.0. This will result in the .watcher-history-11* index without the proper mappings. Without the proper mapping a single document (for a large watch) can exceed the default 1000 field limit and cause error to show in the logs. This commit ensures the same logic for writing to the index is applied as for installing the template. In a mixed cluster, the `10` index template will continue to be written. Only once all of nodes are on 7.7.0+ will the `11` index template be installed and used. closes #56732

elastic#56734) WatcherIndexTemplateRegistry as of elastic#52962 requires all nodes to be on 7.7.0 before it allows the version 11 index template to be installed. While in a mixed cluster, nothing prevents Watcher from running on the new host before the all of the nodes are on 7.7.0. This will result in the .watcher-history-11* index without the proper mappings. Without the proper mapping a single document (for a large watch) can exceed the default 1000 field limit and cause error to show in the logs. This commit ensures the same logic for writing to the index is applied as for installing the template. In a mixed cluster, the `10` index template will continue to be written. Only once all of nodes are on 7.7.0+ will the `11` index template be installed and used. closes elastic#56732

… to use (#56734) (#56855) WatcherIndexTemplateRegistry as of #52962 requires all nodes to be on 7.7.0 before it allows the version 11 index template to be installed. While in a mixed cluster, nothing prevents Watcher from running on the new host before the all of the nodes are on 7.7.0. This will result in the .watcher-history-11* index without the proper mappings. Without the proper mapping a single document (for a large watch) can exceed the default 1000 field limit and cause error to show in the logs. This commit ensures the same logic for writing to the index is applied as for installing the template. In a mixed cluster, the `10` index template will continue to be written. Only once all of nodes are on 7.7.0+ will the `11` index template be installed and used. closes #56732

jakelandis added :Data Management/Watcher v7.8.0 v7.7.1 labels May 14, 2020

jakelandis requested a review from gwbrown May 14, 2020 01:28

elasticmachine added the Team:Data Management Meta label for data/management team label May 14, 2020

jakelandis commented May 14, 2020

View reviewed changes

...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/template/IndexTemplateRegistry.java Show resolved Hide resolved

pugnascotia added the >enhancement label May 14, 2020

jakelandis added >bug and removed >enhancement labels May 14, 2020

gwbrown reviewed May 14, 2020

View reviewed changes

review changes

123453c

Merge branch '7.x' into fix/56732

92a3622

remove premature cluster state read

7a80068

fix precommit

05e2875

gwbrown approved these changes May 15, 2020

View reviewed changes

jakelandis merged commit 813609b into elastic:7.x May 15, 2020

jakelandis deleted the fix/56732 branch May 15, 2020 21:29

jakelandis added the backport pending label May 15, 2020

jakelandis mentioned this pull request May 15, 2020

[7.7] Ensure that .watcher-history-11* template is in installed prior to use (#56734) #56848

Merged

jakelandis added the v7.9.0 label May 15, 2020

jakelandis mentioned this pull request May 15, 2020

[7.8] Ensure that .watcher-history-11* template is in installed prior to use (#56734) #56855

Merged

jakelandis removed the backport pending label May 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that .watcher-history-11* template is in installed prior to use #56734

Ensure that .watcher-history-11* template is in installed prior to use #56734

jakelandis commented May 14, 2020

elasticmachine commented May 14, 2020

jakelandis commented May 14, 2020

gwbrown left a comment

gwbrown May 14, 2020

jakelandis May 15, 2020

jakelandis commented May 15, 2020 •

edited

jakelandis commented May 15, 2020

jakelandis commented May 15, 2020 •

edited

jakelandis commented May 15, 2020

gwbrown left a comment

Ensure that .watcher-history-11* template is in installed prior to use #56734

Ensure that .watcher-history-11* template is in installed prior to use #56734

Conversation

jakelandis commented May 14, 2020

elasticmachine commented May 14, 2020

jakelandis commented May 14, 2020

gwbrown left a comment

Choose a reason for hiding this comment

gwbrown May 14, 2020

Choose a reason for hiding this comment

jakelandis May 15, 2020

Choose a reason for hiding this comment

jakelandis commented May 15, 2020 • edited

jakelandis commented May 15, 2020

jakelandis commented May 15, 2020 • edited

jakelandis commented May 15, 2020

gwbrown left a comment

Choose a reason for hiding this comment

jakelandis commented May 15, 2020 •

edited

jakelandis commented May 15, 2020 •

edited