Skip to content
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.

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

Add a cluster block that allows to delete indices that are read-only #24678

Merged
merged 6 commits into from
May 16, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,20 @@ synchronized ClusterState updateSettings(final ClusterState currentState, Settin
.transientSettings(transientSettings.build());

ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings()) || MetaData.SETTING_READ_ONLY_SETTING.get(metaData.transientSettings());
boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings())
|| MetaData.SETTING_READ_ONLY_SETTING.get(metaData.transientSettings());
if (updatedReadOnly) {
blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
} else {
blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
}
boolean updatedReadOnlyAllowDelete = MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.persistentSettings())
|| MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.transientSettings());
if (updatedReadOnlyAllowDelete) {
blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
} else {
blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
}
ClusterState build = builder(currentState).metaData(metaData).blocks(blocks).build();
Settings settings = build.metaData().settings();
// now we try to apply things and if they are invalid we fail
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ protected String executor() {
@Override
protected ClusterBlockException checkBlock(ClusterUpdateSettingsRequest request, ClusterState state) {
// allow for dedicated changes to the metadata blocks, so we don't block those to allow to "re-enable" it
if ((request.transientSettings().isEmpty() &&
request.persistentSettings().size() == 1 &&
MetaData.SETTING_READ_ONLY_SETTING.exists(request.persistentSettings())) ||
(request.persistentSettings().isEmpty() && request.transientSettings().size() == 1 &&
MetaData.SETTING_READ_ONLY_SETTING.exists(request.transientSettings()))) {
return null;
if (request.transientSettings().size() + request.persistentSettings().size() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fancy pants 😄

// only one setting
if (MetaData.SETTING_READ_ONLY_SETTING.exists(request.persistentSettings())
|| MetaData.SETTING_READ_ONLY_SETTING.exists(request.transientSettings())
|| MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.transientSettings())
|| MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.persistentSettings())) {
// one of the settings above as the only setting in the request means - resetting the block!
return null;
}
}
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ protected void doExecute(Task task, DeleteIndexRequest request, ActionListener<D

@Override
protected ClusterBlockException checkBlock(DeleteIndexRequest request, ClusterState state) {
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request));
return state.blocks().indicesAllowReleaseResources(indexNameExpressionResolver.concreteIndexNames(state, request));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
if (globalBlock != null) {
return globalBlock;
}
if (request.settings().size() == 1 && IndexMetaData.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings()) || IndexMetaData.INDEX_READ_ONLY_SETTING.exists(request.settings())) {
if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock a cluster
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : unblock an index (says cluster in comment)

IndexMetaData.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings())
|| IndexMetaData.INDEX_READ_ONLY_SETTING.exists(request.settings())
|| IndexMetaData.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) {
return null;
}
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.cluster.block;

import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Streamable;
Expand All @@ -43,18 +44,22 @@ public class ClusterBlock implements Streamable, ToXContent {

private boolean disableStatePersistence = false;

private boolean allowReleaseResources;

private RestStatus status;

ClusterBlock() {
}

public ClusterBlock(int id, String description, boolean retryable, boolean disableStatePersistence, RestStatus status, EnumSet<ClusterBlockLevel> levels) {
public ClusterBlock(int id, String description, boolean retryable, boolean disableStatePersistence, RestStatus status,
EnumSet<ClusterBlockLevel> levels, boolean allowReleaseResources) {
this.id = id;
this.description = description;
this.retryable = retryable;
this.disableStatePersistence = disableStatePersistence;
this.status = status;
this.levels = levels;
this.allowReleaseResources = allowReleaseResources;
}

public int id() {
Expand Down Expand Up @@ -127,12 +132,17 @@ public void readFrom(StreamInput in) throws IOException {
final int len = in.readVInt();
ArrayList<ClusterBlockLevel> levels = new ArrayList<>(len);
for (int i = 0; i < len; i++) {
levels.add(ClusterBlockLevel.fromId(in.readVInt()));
levels.add(in.readEnum(ClusterBlockLevel.class));
}
this.levels = EnumSet.copyOf(levels);
retryable = in.readBoolean();
disableStatePersistence = in.readBoolean();
status = RestStatus.readFrom(in);
if (in.getVersion().onOrAfter(Version.V_6_0_0_alpha1_UNRELEASED)) {
allowReleaseResources = in.readBoolean();
} else {
allowReleaseResources = false;
}
}

@Override
Expand All @@ -141,11 +151,14 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(description);
out.writeVInt(levels.size());
for (ClusterBlockLevel level : levels) {
out.writeVInt(level.id());
out.writeEnum(level);
}
out.writeBoolean(retryable);
out.writeBoolean(disableStatePersistence);
RestStatus.writeTo(out, status);
if (out.getVersion().onOrAfter(Version.V_6_0_0_alpha1_UNRELEASED)) {
out.writeBoolean(allowReleaseResources);
}
}

@Override
Expand Down Expand Up @@ -176,4 +189,8 @@ public boolean equals(Object o) {
public int hashCode() {
return id;
}

public boolean isAllowReleaseResources() {
return allowReleaseResources;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,34 +23,11 @@
import java.util.EnumSet;

public enum ClusterBlockLevel {
READ(0),
WRITE(1),
METADATA_READ(2),
METADATA_WRITE(3);
READ,
WRITE,
METADATA_READ,
METADATA_WRITE;

public static final EnumSet<ClusterBlockLevel> ALL = EnumSet.of(READ, WRITE, METADATA_READ, METADATA_WRITE);
public static final EnumSet<ClusterBlockLevel> ALL = EnumSet.allOf(ClusterBlockLevel.class);
public static final EnumSet<ClusterBlockLevel> READ_WRITE = EnumSet.of(READ, WRITE);

private final int id;

ClusterBlockLevel(int id) {
this.id = id;
}

public int id() {
return this.id;
}

static ClusterBlockLevel fromId(int id) {
if (id == 0) {
return READ;
} else if (id == 1) {
return WRITE;
} else if (id == 2) {
return METADATA_READ;
} else if (id == 3) {
return METADATA_WRITE;
}
throw new IllegalArgumentException("No cluster block level matching [" + id + "]");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ public ImmutableOpenMap<String, Set<ClusterBlock>> indices() {
}

public Set<ClusterBlock> global(ClusterBlockLevel level) {
return levelHolders[level.id()].global();
return levelHolders[level.ordinal()].global();
}

public ImmutableOpenMap<String, Set<ClusterBlock>> indices(ClusterBlockLevel level) {
return levelHolders[level.id()].indices();
return levelHolders[level.ordinal()].indices();
}

private Set<ClusterBlock> blocksForIndex(ClusterBlockLevel level, String index) {
Expand All @@ -97,7 +97,7 @@ private static ImmutableLevelHolder[] generateLevelHolders(Set<ClusterBlock> glo
.collect(toSet())));
}

levelHolders[level.id()] = new ImmutableLevelHolder(newGlobal, indicesBuilder.build());
levelHolders[level.ordinal()] = new ImmutableLevelHolder(newGlobal, indicesBuilder.build());
}
return levelHolders;
}
Expand Down Expand Up @@ -203,6 +203,29 @@ public ClusterBlockException indicesBlockedException(ClusterBlockLevel level, St
return new ClusterBlockException(unmodifiableSet(blocks.collect(toSet())));
}

public ClusterBlockException indicesAllowReleaseResources(String[] indices) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can I ask for some java docs? I get the name indicesAllowReleaseResources because I know the context of this change, but I think it's good to give some context as to what it checks.

boolean indexIsBlocked = false;
for (String index : indices) {
if (indexBlocked(ClusterBlockLevel.METADATA_WRITE, index)) {
indexIsBlocked = true;
}
}
if (globalBlocked(ClusterBlockLevel.METADATA_WRITE) == false && indexIsBlocked == false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems indexBlocked already checks for the global blocks, so another check here is redundant. The only exception is if someone passes in an empty array but that is not the intended use of the method.

return null;
}
Function<String, Stream<ClusterBlock>> blocksForIndexAtLevel = index -> blocksForIndex(ClusterBlockLevel.METADATA_WRITE, index)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the first part of the method to shortcut this second half in the case we have no blocks? can't we make do with the second part only?

.stream();
Stream<ClusterBlock> blocks = concat(
global(ClusterBlockLevel.METADATA_WRITE).stream(),
Stream.of(indices).flatMap(blocksForIndexAtLevel)).filter(clusterBlock -> clusterBlock.isAllowReleaseResources() == false);
Set<ClusterBlock> clusterBlocks = unmodifiableSet(blocks.collect(toSet()));
if (clusterBlocks.isEmpty()) {
return null;
}
return new ClusterBlockException(clusterBlocks);
}


@Override
public String toString() {
if (global.isEmpty() && indices().isEmpty()) {
Expand Down Expand Up @@ -270,8 +293,6 @@ public static Diff<ClusterBlocks> readDiffFrom(StreamInput in) throws IOExceptio

static class ImmutableLevelHolder {

static final ImmutableLevelHolder EMPTY = new ImmutableLevelHolder(emptySet(), ImmutableOpenMap.of());

private final Set<ClusterBlock> global;
private final ImmutableOpenMap<String, Set<ClusterBlock>> indices;

Expand Down Expand Up @@ -314,30 +335,31 @@ public Builder blocks(ClusterBlocks blocks) {
}

public Builder addBlocks(IndexMetaData indexMetaData) {
String indexName = indexMetaData.getIndex().getName();
if (indexMetaData.getState() == IndexMetaData.State.CLOSE) {
addIndexBlock(indexMetaData.getIndex().getName(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK);
addIndexBlock(indexName, MetaDataIndexStateService.INDEX_CLOSED_BLOCK);
}
if (IndexMetaData.INDEX_READ_ONLY_SETTING.get(indexMetaData.getSettings())) {
addIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_READ_ONLY_BLOCK);
addIndexBlock(indexName, IndexMetaData.INDEX_READ_ONLY_BLOCK);
}
if (IndexMetaData.INDEX_BLOCKS_READ_SETTING.get(indexMetaData.getSettings())) {
addIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_READ_BLOCK);
addIndexBlock(indexName, IndexMetaData.INDEX_READ_BLOCK);
}
if (IndexMetaData.INDEX_BLOCKS_WRITE_SETTING.get(indexMetaData.getSettings())) {
addIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_WRITE_BLOCK);
addIndexBlock(indexName, IndexMetaData.INDEX_WRITE_BLOCK);
}
if (IndexMetaData.INDEX_BLOCKS_METADATA_SETTING.get(indexMetaData.getSettings())) {
addIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_METADATA_BLOCK);
addIndexBlock(indexName, IndexMetaData.INDEX_METADATA_BLOCK);
}
if (IndexMetaData.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.get(indexMetaData.getSettings())) {
addIndexBlock(indexName, IndexMetaData.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK);
}
return this;
}

public Builder updateBlocks(IndexMetaData indexMetaData) {
removeIndexBlock(indexMetaData.getIndex().getName(), MetaDataIndexStateService.INDEX_CLOSED_BLOCK);
removeIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_READ_ONLY_BLOCK);
removeIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_READ_BLOCK);
removeIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_WRITE_BLOCK);
removeIndexBlock(indexMetaData.getIndex().getName(), IndexMetaData.INDEX_METADATA_BLOCK);
// let's remove all blocks for this index and add them back -- no need to remove all individual blocks....
indices.remove(indexMetaData.getIndex().getName());
return addBlocks(indexMetaData);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,11 @@ public static <T extends Custom> T lookupPrototypeSafe(String type) {
return proto;
}

public static final ClusterBlock INDEX_READ_ONLY_BLOCK = new ClusterBlock(5, "index read-only (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE));
public static final ClusterBlock INDEX_READ_BLOCK = new ClusterBlock(7, "index read (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.READ));
public static final ClusterBlock INDEX_WRITE_BLOCK = new ClusterBlock(8, "index write (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE));
public static final ClusterBlock INDEX_METADATA_BLOCK = new ClusterBlock(9, "index metadata (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.METADATA_READ));
public static final ClusterBlock INDEX_READ_ONLY_BLOCK = new ClusterBlock(5, "index read-only (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE), false);
public static final ClusterBlock INDEX_READ_BLOCK = new ClusterBlock(7, "index read (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.READ), false);
public static final ClusterBlock INDEX_WRITE_BLOCK = new ClusterBlock(8, "index write (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE), false);
public static final ClusterBlock INDEX_METADATA_BLOCK = new ClusterBlock(9, "index metadata (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.METADATA_READ), false);
public static final ClusterBlock INDEX_READ_ONLY_ALLOW_DELETE_BLOCK = new ClusterBlock(12, "index read-only / allow delete (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.WRITE), true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if the booleans were together... but this is really a nit... so keep as is if you prefer it like this.


public enum State {
OPEN((byte) 0),
Expand Down Expand Up @@ -212,6 +213,10 @@ static Setting<Integer> buildNumberOfShardsSetting() {
public static final Setting<Boolean> INDEX_BLOCKS_METADATA_SETTING =
Setting.boolSetting(SETTING_BLOCKS_METADATA, false, Property.Dynamic, Property.IndexScope);

public static final String SETTING_READ_ONLY_ALLOW_DELETE = "index.blocks.read_only_allow_delete";
public static final Setting<Boolean> INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING =
Setting.boolSetting(SETTING_READ_ONLY_ALLOW_DELETE, false, Property.Dynamic, Property.IndexScope);

public static final String SETTING_VERSION_CREATED = "index.version.created";
public static final String SETTING_VERSION_CREATED_STRING = "index.version.created_string";
public static final String SETTING_VERSION_UPGRADED = "index.version.upgraded";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.Diff;
import org.elasticsearch.cluster.Diffable;
import org.elasticsearch.cluster.DiffableUtils;
Expand Down Expand Up @@ -119,7 +118,14 @@ public interface Custom extends NamedDiffable<Custom>, ToXContent {
public static final Setting<Boolean> SETTING_READ_ONLY_SETTING =
Setting.boolSetting("cluster.blocks.read_only", false, Property.Dynamic, Property.NodeScope);

public static final ClusterBlock CLUSTER_READ_ONLY_BLOCK = new ClusterBlock(6, "cluster read-only (api)", false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE));
public static final ClusterBlock CLUSTER_READ_ONLY_BLOCK = new ClusterBlock(6, "cluster read-only (api)", false, false,
RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE), false);

public static final Setting<Boolean> SETTING_READ_ONLY_ALLOW_DELETE_SETTING =
Setting.boolSetting("cluster.blocks.read_only_allow_delete", false, Property.Dynamic, Property.NodeScope);

public static final ClusterBlock CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK = new ClusterBlock(13, "cluster read-only / allow delete (api)",
false, false, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.WRITE, ClusterBlockLevel.METADATA_WRITE), true);

public static final MetaData EMPTY_META_DATA = builder().build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
*/
public class MetaDataIndexStateService extends AbstractComponent {

public static final ClusterBlock INDEX_CLOSED_BLOCK = new ClusterBlock(4, "index closed", false, false, RestStatus.FORBIDDEN, ClusterBlockLevel.READ_WRITE);
public static final ClusterBlock INDEX_CLOSED_BLOCK = new ClusterBlock(4, "index closed", false, false, RestStatus.FORBIDDEN, ClusterBlockLevel.READ_WRITE, false);

private final ClusterService clusterService;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ public ClusterState execute(ClusterState currentState) {

ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
maybeUpdateClusterBlock(actualIndices, blocks, IndexMetaData.INDEX_READ_ONLY_BLOCK, IndexMetaData.INDEX_READ_ONLY_SETTING, openSettings);
maybeUpdateClusterBlock(actualIndices, blocks, IndexMetaData.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK, IndexMetaData.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING, openSettings);
maybeUpdateClusterBlock(actualIndices, blocks, IndexMetaData.INDEX_METADATA_BLOCK, IndexMetaData.INDEX_BLOCKS_METADATA_SETTING, openSettings);
maybeUpdateClusterBlock(actualIndices, blocks, IndexMetaData.INDEX_WRITE_BLOCK, IndexMetaData.INDEX_BLOCKS_WRITE_SETTING, openSettings);
maybeUpdateClusterBlock(actualIndices, blocks, IndexMetaData.INDEX_READ_BLOCK, IndexMetaData.INDEX_BLOCKS_READ_SETTING, openSettings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ public void apply(Settings value, Settings current, Settings previous) {
IndicesQueryCache.INDICES_QUERIES_CACHE_ALL_SEGMENTS_SETTING,
MappingUpdatedAction.INDICES_MAPPING_DYNAMIC_TIMEOUT_SETTING,
MetaData.SETTING_READ_ONLY_SETTING,
MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING,
RecoverySettings.INDICES_RECOVERY_MAX_BYTES_PER_SEC_SETTING,
RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_STATE_SYNC_SETTING,
RecoverySettings.INDICES_RECOVERY_RETRY_DELAY_NETWORK_SETTING,
Expand Down
Loading