Skip to content

Commit 0e6cc6f

Browse files
dnhatndnhatn
committed
Skip update if leader and follower settings identical (#44535)
If the setting on the follower and the leader are identical after filtering out private and internal settings, then we should not call update setting (on the follower) as there's nothing to change. Moreover, this makes the ShardFollowTask abort as it considers ActionRequestValidationException (caused by an empty update setting request) as a fatal error. Closes #44521
1 parent 8ca57c9 commit 0e6cc6f

File tree

2 files changed

+115
-4
lines changed

2 files changed

+115
-4
lines changed

‎x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTasksExecutor.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.elasticsearch.cluster.service.ClusterService;
3232
import org.elasticsearch.common.CheckedConsumer;
3333
import org.elasticsearch.common.settings.IndexScopedSettings;
34+
import org.elasticsearch.common.settings.Setting;
3435
import org.elasticsearch.common.settings.Settings;
3536
import org.elasticsearch.common.settings.SettingsModule;
3637
import org.elasticsearch.common.unit.TimeValue;
@@ -182,10 +183,17 @@ protected void innerUpdateSettings(final LongConsumer finalHandler, final Consum
182183
finalHandler.accept(leaderIMD.getSettingsVersion());
183184
} else {
184185
// Figure out which settings have been updated:
185-
final Settings updatedSettings = settings.filter(
186-
s -> existingSettings.get(s) == null || existingSettings.get(s).equals(settings.get(s)) == false
187-
);
188-
186+
final Settings updatedSettings = settings.filter(s -> {
187+
final Setting<?> indexSettings = indexScopedSettings.get(s);
188+
if (indexSettings == null || indexSettings.isPrivateIndex() || indexSettings.isInternalIndex()) {
189+
return false;
190+
}
191+
return existingSettings.get(s) == null || existingSettings.get(s).equals(settings.get(s)) == false;
192+
});
193+
if (updatedSettings.isEmpty()) {
194+
finalHandler.accept(leaderIMD.getSettingsVersion());
195+
return;
196+
}
189197
// Figure out whether the updated settings are all dynamic settings and
190198
// if so just update the follower index's settings:
191199
if (updatedSettings.keySet().stream().allMatch(indexScopedSettings::isDynamicSetting)) {

‎x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/IndexFollowingIT.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package org.elasticsearch.xpack.ccr;
88

99
import com.carrotsearch.hppc.cursors.ObjectCursor;
10+
import org.apache.lucene.util.SetOnce;
1011
import org.elasticsearch.ElasticsearchException;
1112
import org.elasticsearch.ElasticsearchStatusException;
1213
import org.elasticsearch.ExceptionsHelper;
@@ -41,10 +42,12 @@
4142
import org.elasticsearch.action.support.WriteRequest;
4243
import org.elasticsearch.client.Requests;
4344
import org.elasticsearch.cluster.ClusterState;
45+
import org.elasticsearch.cluster.ClusterStateUpdateTask;
4446
import org.elasticsearch.cluster.health.ClusterIndexHealth;
4547
import org.elasticsearch.cluster.health.ClusterShardHealth;
4648
import org.elasticsearch.cluster.metadata.IndexMetaData;
4749
import org.elasticsearch.cluster.metadata.MappingMetaData;
50+
import org.elasticsearch.cluster.metadata.MetaData;
4851
import org.elasticsearch.cluster.routing.IndexShardRoutingTable;
4952
import org.elasticsearch.cluster.routing.RoutingTable;
5053
import org.elasticsearch.cluster.service.ClusterService;
@@ -64,6 +67,7 @@
6467
import org.elasticsearch.index.seqno.RetentionLeaseActions;
6568
import org.elasticsearch.index.shard.ShardId;
6669
import org.elasticsearch.persistent.PersistentTasksCustomMetaData;
70+
import org.elasticsearch.plugins.Plugin;
6771
import org.elasticsearch.rest.RestStatus;
6872
import org.elasticsearch.snapshots.SnapshotRestoreException;
6973
import org.elasticsearch.tasks.TaskInfo;
@@ -84,6 +88,7 @@
8488
import org.elasticsearch.xpack.core.ccr.action.UnfollowAction;
8589

8690
import java.io.IOException;
91+
import java.util.Arrays;
8792
import java.util.Collection;
8893
import java.util.Collections;
8994
import java.util.HashMap;
@@ -92,12 +97,14 @@
9297
import java.util.Map;
9398
import java.util.Objects;
9499
import java.util.Set;
100+
import java.util.concurrent.CountDownLatch;
95101
import java.util.concurrent.Semaphore;
96102
import java.util.concurrent.TimeUnit;
97103
import java.util.concurrent.atomic.AtomicBoolean;
98104
import java.util.function.BooleanSupplier;
99105
import java.util.function.Consumer;
100106
import java.util.stream.Collectors;
107+
import java.util.stream.Stream;
101108

102109
import static java.util.Collections.singletonMap;
103110
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -114,6 +121,11 @@
114121

115122
public class IndexFollowingIT extends CcrIntegTestCase {
116123

124+
@Override
125+
protected Collection<Class<? extends Plugin>> nodePlugins() {
126+
return Stream.concat(super.nodePlugins().stream(), Stream.of(PrivateSettingPlugin.class)).collect(Collectors.toList());
127+
}
128+
117129
public void testFollowIndex() throws Exception {
118130
final int numberOfPrimaryShards = randomIntBetween(1, 3);
119131
int numberOfReplicas = between(0, 1);
@@ -965,6 +977,86 @@ public void testUpdateAnalysisLeaderIndexSettings() throws Exception {
965977
assertThat(hasFollowIndexBeenClosedChecker.getAsBoolean(), is(true));
966978
}
967979

980+
public void testDoNotReplicatePrivateSettings() throws Exception {
981+
assertAcked(leaderClient().admin().indices().prepareCreate("leader").setSource(
982+
getIndexSettings(1, 0, singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")), XContentType.JSON));
983+
ensureLeaderGreen("leader");
984+
final PutFollowAction.Request followRequest = putFollow("leader", "follower");
985+
followerClient().execute(PutFollowAction.INSTANCE, followRequest).get();
986+
ClusterService clusterService = getLeaderCluster().getInstance(ClusterService.class, getLeaderCluster().getMasterName());
987+
clusterService.submitStateUpdateTask("test", new ClusterStateUpdateTask() {
988+
@Override
989+
public ClusterState execute(ClusterState currentState) {
990+
final IndexMetaData indexMetaData = currentState.metaData().index("leader");
991+
Settings.Builder settings = Settings.builder()
992+
.put(indexMetaData.getSettings())
993+
.put("index.max_ngram_diff", 2);
994+
if (randomBoolean()) {
995+
settings.put(PrivateSettingPlugin.INDEX_INTERNAL_SETTING.getKey(), "private-value");
996+
}
997+
if (randomBoolean()) {
998+
settings.put(PrivateSettingPlugin.INDEX_PRIVATE_SETTING.getKey(), "interval-value");
999+
}
1000+
final MetaData.Builder metadata = MetaData.builder(currentState.metaData())
1001+
.put(IndexMetaData.builder(indexMetaData)
1002+
.settingsVersion(indexMetaData.getSettingsVersion() + 1)
1003+
.settings(settings).build(), true);
1004+
return ClusterState.builder(currentState).metaData(metadata).build();
1005+
}
1006+
1007+
@Override
1008+
public void onFailure(String source, Exception e) {
1009+
throw new AssertionError(e);
1010+
}
1011+
});
1012+
assertBusy(() -> {
1013+
GetSettingsResponse resp = followerClient().admin().indices().prepareGetSettings("follower").get();
1014+
assertThat(resp.getSetting("follower", "index.max_ngram_diff"), equalTo("2"));
1015+
assertThat(resp.getSetting("follower", PrivateSettingPlugin.INDEX_INTERNAL_SETTING.getKey()), nullValue());
1016+
assertThat(resp.getSetting("follower", PrivateSettingPlugin.INDEX_PRIVATE_SETTING.getKey()), nullValue());
1017+
});
1018+
}
1019+
1020+
public void testReplicatePrivateSettingsOnly() throws Exception {
1021+
assertAcked(leaderClient().admin().indices().prepareCreate("leader").setSource(
1022+
getIndexSettings(1, 0, singletonMap(IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(), "true")), XContentType.JSON));
1023+
ensureLeaderGreen("leader");
1024+
followerClient().execute(PutFollowAction.INSTANCE, putFollow("leader", "follower")).get();
1025+
final ClusterService clusterService = getLeaderCluster().getInstance(ClusterService.class, getLeaderCluster().getMasterName());
1026+
final SetOnce<Long> settingVersionOnLeader = new SetOnce<>();
1027+
final CountDownLatch latch = new CountDownLatch(1);
1028+
clusterService.submitStateUpdateTask("test", new ClusterStateUpdateTask() {
1029+
@Override
1030+
public ClusterState execute(ClusterState currentState) {
1031+
final IndexMetaData indexMetaData = currentState.metaData().index("leader");
1032+
Settings.Builder settings = Settings.builder().put(indexMetaData.getSettings());
1033+
settings.put(PrivateSettingPlugin.INDEX_PRIVATE_SETTING.getKey(), "internal-value");
1034+
settings.put(PrivateSettingPlugin.INDEX_INTERNAL_SETTING.getKey(), "internal-value");
1035+
final MetaData.Builder metadata = MetaData.builder(currentState.metaData())
1036+
.put(IndexMetaData.builder(indexMetaData)
1037+
.settingsVersion(indexMetaData.getSettingsVersion() + 1)
1038+
.settings(settings).build(), true);
1039+
return ClusterState.builder(currentState).metaData(metadata).build();
1040+
}
1041+
1042+
@Override
1043+
public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
1044+
settingVersionOnLeader.set(newState.metaData().index("leader").getSettingsVersion());
1045+
latch.countDown();
1046+
}
1047+
1048+
@Override
1049+
public void onFailure(String source, Exception e) {
1050+
throw new AssertionError(e);
1051+
}
1052+
});
1053+
latch.await();
1054+
assertBusy(() -> assertThat(getFollowTaskSettingsVersion("follower"), equalTo(settingVersionOnLeader.get())));
1055+
GetSettingsResponse resp = followerClient().admin().indices().prepareGetSettings("follower").get();
1056+
assertThat(resp.getSetting("follower", PrivateSettingPlugin.INDEX_INTERNAL_SETTING.getKey()), nullValue());
1057+
assertThat(resp.getSetting("follower", PrivateSettingPlugin.INDEX_PRIVATE_SETTING.getKey()), nullValue());
1058+
}
1059+
9681060
public void testMustCloseIndexAndPauseToRestartWithPutFollowing() throws Exception {
9691061
final int numberOfPrimaryShards = randomIntBetween(1, 3);
9701062
final String leaderIndexSettings = getIndexSettings(numberOfPrimaryShards, between(0, 1),
@@ -1332,4 +1424,15 @@ private String getIndexSettingsWithNestedMapping(final int numberOfShards, final
13321424
return settings;
13331425
}
13341426

1427+
public static class PrivateSettingPlugin extends Plugin {
1428+
static final Setting<String> INDEX_INTERNAL_SETTING =
1429+
Setting.simpleString("index.internal", Setting.Property.IndexScope, Setting.Property.InternalIndex);
1430+
static final Setting<String> INDEX_PRIVATE_SETTING =
1431+
Setting.simpleString("index.private", Setting.Property.IndexScope, Setting.Property.PrivateIndex);
1432+
1433+
@Override
1434+
public List<Setting<?>> getSettings() {
1435+
return Arrays.asList(INDEX_INTERNAL_SETTING, INDEX_PRIVATE_SETTING);
1436+
}
1437+
}
13351438
}

0 commit comments

Comments
 (0)