Detect and optimize noop of update index settings (#61348)

This optimization is more relevant in the context of CCR. When a node in
the follower cluster leaves, we reallocate the shard-follow tasks on 
that node to other nodes. The new tasks will overwhelm the follower
cluster with many put-mapping, update-settings requests, although most
of them are noop. This change detects and optimizes the noop
update-settings requests.
This commit is contained in:
Nhat Nguyen 2020-08-20 15:56:16 -04:00
parent 439fa46735
commit 23a0f8b617
3 changed files with 78 additions and 8 deletions

View File

@ -21,10 +21,13 @@ package org.elasticsearch.indices.settings;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority; import org.elasticsearch.common.Priority;
import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.IndexModule; import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.VersionType; import org.elasticsearch.index.VersionType;
@ -686,4 +689,52 @@ public class UpdateSettingsIT extends ESIntegTestCase {
assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1)); assertThat(IndexMetadata.INDEX_NUMBER_OF_REPLICAS_SETTING.get(response.getIndexToSettings().get("test")), equalTo(1));
} }
public void testNoopUpdate() {
internalCluster().ensureAtLeastNumDataNodes(2);
final ClusterService clusterService = internalCluster().getMasterNodeInstance(ClusterService.class);
assertAcked(client().admin().indices().prepareCreate("test")
.setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)));
ClusterState currentState = clusterService.state();
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)));
assertNotSame(currentState, clusterService.state());
client().admin().cluster().prepareHealth()
.setWaitForGreenStatus()
.setWaitForNoInitializingShards(true)
.setWaitForEvents(Priority.LANGUID)
.setTimeout(TimeValue.MAX_VALUE)
.get();
currentState = clusterService.state();
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)));
assertSame(clusterService.state(), currentState);
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull(IndexMetadata.SETTING_NUMBER_OF_REPLICAS)));
assertSame(clusterService.state(), currentState);
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder()
.putNull(SETTING_BLOCKS_READ)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)));
assertSame(currentState, clusterService.state());
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder()
.put(SETTING_BLOCKS_READ, true)
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)));
assertNotSame(currentState, clusterService.state());
currentState = clusterService.state();
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().put(SETTING_BLOCKS_READ, true)));
assertSame(currentState, clusterService.state());
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull(SETTING_BLOCKS_READ)));
assertNotSame(currentState, clusterService.state());
}
} }

View File

@ -31,6 +31,7 @@ import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.RestStatus;
import java.io.IOException; import java.io.IOException;
import java.util.Collections;
import java.util.EnumMap; import java.util.EnumMap;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
@ -405,6 +406,10 @@ public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> {
return this; return this;
} }
public boolean hasIndexBlock(String index, ClusterBlock block) {
return indices.getOrDefault(index, Collections.emptySet()).contains(block);
}
public Builder removeIndexBlock(String index, ClusterBlock block) { public Builder removeIndexBlock(String index, ClusterBlock block) {
if (!indices.containsKey(index)) { if (!indices.containsKey(index)) {
return this; return this;

View File

@ -177,11 +177,6 @@ public class MetadataUpdateSettingsService {
} }
} }
ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
for (IndexMetadata.APIBlock block : IndexMetadata.APIBlock.values()) {
maybeUpdateClusterBlock(actualIndices, blocks, block.block, block.setting, openSettings);
}
if (!openIndices.isEmpty()) { if (!openIndices.isEmpty()) {
for (Index index : openIndices) { for (Index index : openIndices) {
IndexMetadata indexMetadata = metadataBuilder.getSafe(index); IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
@ -246,15 +241,26 @@ public class MetadataUpdateSettingsService {
MetadataCreateIndexService.validateTranslogRetentionSettings(metadataBuilder.get(index).getSettings()); MetadataCreateIndexService.validateTranslogRetentionSettings(metadataBuilder.get(index).getSettings());
} }
} }
boolean changed = false;
// increment settings versions // increment settings versions
for (final String index : actualIndices) { for (final String index : actualIndices) {
if (same(currentState.metadata().index(index).getSettings(), metadataBuilder.get(index).getSettings()) == false) { if (same(currentState.metadata().index(index).getSettings(), metadataBuilder.get(index).getSettings()) == false) {
changed = true;
final IndexMetadata.Builder builder = IndexMetadata.builder(metadataBuilder.get(index)); final IndexMetadata.Builder builder = IndexMetadata.builder(metadataBuilder.get(index));
builder.settingsVersion(1 + builder.settingsVersion()); builder.settingsVersion(1 + builder.settingsVersion());
metadataBuilder.put(builder); metadataBuilder.put(builder);
} }
} }
final ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
for (IndexMetadata.APIBlock block : IndexMetadata.APIBlock.values()) {
changed |= maybeUpdateClusterBlock(actualIndices, blocks, block.block, block.setting, openSettings);
}
if (changed == false) {
return currentState;
}
ClusterState updatedState = ClusterState.builder(currentState).metadata(metadataBuilder) ClusterState updatedState = ClusterState.builder(currentState).metadata(metadataBuilder)
.routingTable(routingTableBuilder.build()).blocks(blocks).build(); .routingTable(routingTableBuilder.build()).blocks(blocks).build();
@ -294,19 +300,27 @@ public class MetadataUpdateSettingsService {
/** /**
* Updates the cluster block only iff the setting exists in the given settings * Updates the cluster block only iff the setting exists in the given settings
*/ */
private static void maybeUpdateClusterBlock(String[] actualIndices, ClusterBlocks.Builder blocks, ClusterBlock block, private static boolean maybeUpdateClusterBlock(String[] actualIndices, ClusterBlocks.Builder blocks, ClusterBlock block,
Setting<Boolean> setting, Settings openSettings) { Setting<Boolean> setting, Settings openSettings) {
boolean changed = false;
if (setting.exists(openSettings)) { if (setting.exists(openSettings)) {
final boolean updateBlock = setting.get(openSettings); final boolean updateBlock = setting.get(openSettings);
for (String index : actualIndices) { for (String index : actualIndices) {
if (updateBlock) { if (updateBlock) {
if (blocks.hasIndexBlock(index, block) == false) {
blocks.addIndexBlock(index, block); blocks.addIndexBlock(index, block);
changed = true;
}
} else { } else {
if (blocks.hasIndexBlock(index, block)) {
blocks.removeIndexBlock(index, block); blocks.removeIndexBlock(index, block);
changed = true;
} }
} }
} }
} }
return changed;
}
public void upgradeIndexSettings(final UpgradeSettingsClusterStateUpdateRequest request, public void upgradeIndexSettings(final UpgradeSettingsClusterStateUpdateRequest request,