Replicate index settings to followers

[jasontedor]

This commit uses the index settings version so that a follower can replicate index settings changes as needed from the leader.

[elasticmachine]

Pinging @elastic/es-distributed

[jasontedor]

This code is really a POC, there is some work to be done to make this production-ready, and tests are needed. This effort will be picked up by @martijnvg.

[martijnvg]

@jasontedor I think I have made this change production ready.

The most significant change is that the PersistentTaskPlugin interface has changed. This was needed to pass down IndexScopedSettings to ShardFollowTasksExecutor in order to determine whether index settings are dynamic.

[martijnvg]

@elasticmachine run gradle build test

[martijnvg]

run package tests please

[jasontedor]

I left a few comments. 😉

[jasontedor]

Pushing SettingsModule would feel better to me. IndexScopedSettings feels too specific exactly to CCR needs.

[jasontedor]

Maybe:

diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java
index 6cef9689409..2049bf39989 100644
--- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java
+++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java
@@ -170,16 +170,7 @@ public class ShardFollowTasksExecutor extends PersistentTasksExecutor<ShardFollo
                             s -> existingSettings.get(s) == null || existingSettings.get(s).equals(settings.get(s)) == false
                         );
 
-                        // Figure out whether the updated settings are all dynamic settings:
-                        boolean onlyDynamicSettings = true;
-                        for (String key : updatedSettings.keySet()) {
-                            if (indexScopedSettings.isDynamicSetting(key) == false) {
-                                onlyDynamicSettings = false;
-                                break;
-                            }
-                        }
-
-                        if (onlyDynamicSettings) {
+                        if (updatedSettings.keySet().stream().allMatch(indexScopedSettings::isDynamicSetting)) {
                             // If only dynamic settings have been updated then just update these settings in follower index:
                             final UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(followIndex.getName());
                             updateSettingsRequest.settings(updatedSettings);

[jasontedor]

This should probably be named closeIndexUpdateSettingsAndOpenIndex.

[jasontedor]

updateSettingsAndOpenIndex

[martijnvg]

@jasontedor I've addressed the comments.

[jasontedor]

I can't approve this PR. Would you also add a test for updating a setting that shouldn't percolate to the follower (so that the setting version indeed changes, but there's no settings change on the follower). Then I am good with this PR being merged.

Thank you for all the work here, and taking my POC to the finish line. This is great work by you (really, I gave you something rough, you polished it really nicely, and made it production-ready). 🙏

[martijnvg]

Thanks @jasontedor for reviewing!

Would you also add a test for updating a setting that shouldn't percolate to the follower (so that the setting version indeed changes, but there's no settings change on the follower)

I think the testUpdateWhiteListedLeaderIndexSettings() test is already testing this? It sets the index.number_of_replicas on leader index and verifies that that setting does not percolate to the follower index, but it does expect that the settings version is incremented on the follower index.

I'll rename this test the better reflect what it is testing.

[jasontedor]

Thanks for clarifying that test.

[jasontedor]

I can't approve this. LGTM.