Add a cluster block that allows to delete indices that are read-only #24678
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today when an index is `read-only` the index is also blocked from being deleted which sometimes is undesired since in-order to make changes to a cluster indices must be deleted to free up space. This is a likely scenario in a hosted enviroment when disk-space is limited to switch indices read-only but allow deletions to free up space.
s1monw
added
:Cluster
:Core/Infra/Settings
|
Relates to #24299 |
bleskes
approved these changes
May 16, 2017
| (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) { |
| @@ -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 | |||
There was a problem hiding this comment.
nit : unblock an index (says cluster in comment)
| @@ -203,6 +203,29 @@ public ClusterBlockException indicesBlockedException(ClusterBlockLevel level, St | |||
| return new ClusterBlockException(unmodifiableSet(blocks.collect(toSet()))); | |||
| } | |||
|
|
|||
| public ClusterBlockException indicesAllowReleaseResources(String[] indices) { | |||
There was a problem hiding this comment.
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.
| indexIsBlocked = true; | ||
| } | ||
| } | ||
| if (globalBlocked(ClusterBlockLevel.METADATA_WRITE) == false && indexIsBlocked == false) { |
There was a problem hiding this comment.
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.
| if (globalBlocked(ClusterBlockLevel.METADATA_WRITE) == false && indexIsBlocked == false) { | ||
| return null; | ||
| } | ||
| Function<String, Stream<ClusterBlock>> blocksForIndexAtLevel = index -> blocksForIndex(ClusterBlockLevel.METADATA_WRITE, index) |
There was a problem hiding this comment.
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?
| 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); |
There was a problem hiding this comment.
I would prefer if the booleans were together... but this is really a nit... so keep as is if you prefer it like this.
| Settings settings = Settings.builder().put(IndexMetaData.SETTING_READ_ONLY_ALLOW_DELETE, true).build(); | ||
| assertAcked(client().admin().indices().prepareUpdateSettings("test").setSettings(settings).get()); | ||
| assertSearchHits(client().prepareSearch().get(), "1"); | ||
| assertBlocked(client().prepareIndex().setIndex("test").setType("doc").setId("2").setSource("foo", "bar"), |
There was a problem hiding this comment.
can we also check that metadata changes are rejected?
| assertAcked(client().admin().cluster().prepareUpdateSettings().setTransientSettings(settings).get()); | ||
|
|
||
| } finally { | ||
| Settings s = Settings.builder().putNull(MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.getKey()).build(); |
There was a problem hiding this comment.
this is funky - we can remove the previous non finaly logic
|
@bleskes thanks for the review - I addressed all the comments |
s1monw
added a commit
that referenced
this pull request
May 16, 2017
…24678) Today when an index is `read-only` the index is also blocked from being deleted which sometimes is undesired since in-order to make changes to a cluster indices must be deleted to free up space. This is a likely scenario in a hosted environment when disk-space is limited to switch indices read-only but allow deletions to free up space.
clintongormley
added
:Distributed Indexing/Distributed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today when an index is
read-onlythe index is also blocked frombeing deleted which sometimes is undesired since in-order to make
changes to a cluster indices must be deleted to free up space. This is
a likely scenario in a hosted environment when disk-space is limited to switch
indices read-only but allow deletions to free up space.