Skip to content

Detect noop of update index settings #61348

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 20, 2020
Merged

Detect noop of update index settings #61348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0729616
Detect noop of index setting update
dnhatn Aug 19, 2020
b5d5f3b
Merge branch 'master' into noop-update-index
dnhatn Aug 20, 2020
f43c2ea
detect noop in blocks
dnhatn Aug 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions server/src/internalClusterTest/java/org/elasticsearch/indices/settings/UpdateSettingsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@

import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.VersionType;
Expand Down Expand Up @@ -687,4 +690,52 @@ private void runTestDefaultNumberOfReplicasTest(final boolean closeIndex) {
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());
}

}
5 changes: 5 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/block/ClusterBlocks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -405,6 +406,10 @@ public Builder removeIndexBlocks(String index) {
return this;
}

public boolean hasIndexBlock(String index, ClusterBlock block) {
return indices.getOrDefault(index, Collections.emptySet()).contains(block);
}

public Builder removeIndexBlock(String index, ClusterBlock block) {
if (!indices.containsKey(index)) {
return this;
Expand Down
30 changes: 22 additions & 8 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,6 @@ public ClusterState execute(ClusterState currentState) {
}
}

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()) {
for (Index index : openIndices) {
IndexMetadata indexMetadata = metadataBuilder.getSafe(index);
Expand Down Expand Up @@ -246,15 +241,26 @@ public ClusterState execute(ClusterState currentState) {
MetadataCreateIndexService.validateTranslogRetentionSettings(metadataBuilder.get(index).getSettings());
}
}
boolean changed = false;
// increment settings versions
for (final String index : actualIndices) {
if (same(currentState.metadata().index(index).getSettings(), metadataBuilder.get(index).getSettings()) == false) {
changed = true;
final IndexMetadata.Builder builder = IndexMetadata.builder(metadataBuilder.get(index));
builder.settingsVersion(1 + builder.settingsVersion());
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)
.routingTable(routingTableBuilder.build()).blocks(blocks).build();

Expand Down Expand Up @@ -294,18 +300,26 @@ private int getTotalNewShards(Index index, ClusterState currentState, int update
/**
* 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) {
boolean changed = false;
if (setting.exists(openSettings)) {
final boolean updateBlock = setting.get(openSettings);
for (String index : actualIndices) {
if (updateBlock) {
blocks.addIndexBlock(index, block);
if (blocks.hasIndexBlock(index, block) == false) {
blocks.addIndexBlock(index, block);
changed = true;
}
} else {
blocks.removeIndexBlock(index, block);
if (blocks.hasIndexBlock(index, block)) {
blocks.removeIndexBlock(index, block);
changed = true;
}
}
}
}
return changed;
}


Expand Down