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

Return 429 status code when there's a read_only cluster block #50166

Merged
merged 17 commits into from Feb 22, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -110,13 +110,23 @@ private static String buildMessageForIndexBlocks(Map<String, Set<ClusterBlock>>
@Override
public RestStatus status() {
RestStatus status = null;
boolean onlyRetryableBlocks = true;
for (ClusterBlock block : blocks) {
if (status == null) {
status = block.status();
} else if (status.getStatus() < block.status().getStatus()) {
status = block.status();
boolean isRetryableBlock = block.status() == RestStatus.TOO_MANY_REQUESTS;
if (!isRetryableBlock) {
gaobinlong marked this conversation as resolved.
Show resolved Hide resolved
if (status == null) {
status = block.status();
} else if (status.getStatus() < block.status().getStatus()) {
status = block.status();
}
}
onlyRetryableBlocks = onlyRetryableBlocks && isRetryableBlock;
}
// return retryable status if there are only retryable blocks
if (onlyRetryableBlocks) {
return RestStatus.TOO_MANY_REQUESTS;
}
// return status which has the maximum code of all status except the retryable blocks'
return status;
}
}
Expand Up @@ -95,7 +95,7 @@ public class IndexMetaData implements Diffable<IndexMetaData>, ToXContentFragmen
RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.METADATA_READ));
public static final ClusterBlock INDEX_READ_ONLY_ALLOW_DELETE_BLOCK =
new ClusterBlock(12, "index read-only / allow delete (api)", false, false,
true, RestStatus.FORBIDDEN, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.WRITE));
true, RestStatus.TOO_MANY_REQUESTS, EnumSet.of(ClusterBlockLevel.METADATA_WRITE, ClusterBlockLevel.WRITE));

public enum State {
OPEN((byte) 0),
Expand Down
Expand Up @@ -72,7 +72,7 @@ public void testClusterBlockMessageHasIndexName() {
client().admin().indices().prepareUpdateSettings("test").setSettings(settings).get();
ClusterBlockException e = expectThrows(ClusterBlockException.class, () ->
client().prepareIndex().setIndex("test").setId("1").setSource("foo", "bar").get());
assertEquals("index [test] blocked by: [FORBIDDEN/12/index read-only / allow delete (api)];", e.getMessage());
assertEquals("index [test] blocked by: [TOO_MANY_REQUESTS/12/index read-only / allow delete (api)];", e.getMessage());
} finally {
assertAcked(client().admin().indices().prepareUpdateSettings("test")
.setSettings(Settings.builder().putNull(IndexMetaData.SETTING_READ_ONLY_ALLOW_DELETE).build()).get());
Expand Down
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.cluster.block.ClusterBlock;
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesReference;
Expand Down Expand Up @@ -148,7 +149,9 @@ public static void assertBlocked(BroadcastResponse replicatedBroadcastResponse)
assertNotNull("expected the cause of failure to be a ClusterBlockException but got " + exception.getCause().getMessage(),
clusterBlockException);
assertThat(clusterBlockException.blocks().size(), greaterThan(0));
assertThat(clusterBlockException.status(), CoreMatchers.equalTo(RestStatus.FORBIDDEN));

RestStatus status = checkRetryableBlock(clusterBlockException.blocks()) ? RestStatus.TOO_MANY_REQUESTS : RestStatus.FORBIDDEN;
assertThat(clusterBlockException.status(), CoreMatchers.equalTo(status));
}
}

Expand All @@ -164,7 +167,8 @@ public static void assertBlocked(final ActionRequestBuilder builder, @Nullable f
fail("Request executed with success but a ClusterBlockException was expected");
} catch (ClusterBlockException e) {
assertThat(e.blocks().size(), greaterThan(0));
assertThat(e.status(), equalTo(RestStatus.FORBIDDEN));
RestStatus status = checkRetryableBlock(e.blocks()) ? RestStatus.TOO_MANY_REQUESTS : RestStatus.FORBIDDEN;
assertThat(e.status(), equalTo(status));

if (expectedBlockId != null) {
boolean found = false;
Expand All @@ -189,6 +193,16 @@ public static void assertBlocked(final ActionRequestBuilder builder, @Nullable f
assertBlocked(builder, expectedBlock != null ? expectedBlock.id() : null);
}

private static boolean checkRetryableBlock(Set<ClusterBlock> clusterBlocks){
// check only retryable blocks exist in the set
for (ClusterBlock clusterBlock : clusterBlocks) {
if (clusterBlock.id() != IndexMetaData.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK.id()) {
return false;
}
}
return true;
}

public static String formatShardStatus(BroadcastResponse response) {
StringBuilder msg = new StringBuilder();
msg.append(" Total shards: ").append(response.getTotalShards())
Expand Down
Expand Up @@ -19,6 +19,11 @@

package org.elasticsearch.test.hamcrest;

import org.elasticsearch.action.support.DefaultShardOperationFailedException;
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
import org.elasticsearch.cluster.block.ClusterBlock;
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand All @@ -27,7 +32,14 @@
import org.elasticsearch.test.RandomObjects;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
import static org.hamcrest.Matchers.containsString;

Expand Down Expand Up @@ -188,4 +200,49 @@ public void testAssertXContentEquivalentErrors() throws IOException {
assertThat(error.getMessage(), containsString("expected [1] more entries"));
}
}

public void testAssertBlocked() {
{
gaobinlong marked this conversation as resolved.
Show resolved Hide resolved
List<DefaultShardOperationFailedException> shardFailures = new ArrayList<>();
Set<ClusterBlock> clusterBlocks = new HashSet<>();
clusterBlocks.add(IndexMetaData.INDEX_READ_ONLY_BLOCK);
gaobinlong marked this conversation as resolved.
Show resolved Hide resolved
Map<String, Set<ClusterBlock>> indexLevelBlocks = new HashMap<>();
indexLevelBlocks.put("test", clusterBlocks);
shardFailures.add(new DefaultShardOperationFailedException("test", 0, new ClusterBlockException(indexLevelBlocks)));
BroadcastResponse broadcastResponse = new BroadcastResponse(1, 0, 1, shardFailures);
assertBlocked(broadcastResponse);
}
{
List<DefaultShardOperationFailedException> shardFailures = new ArrayList<>();
Set<ClusterBlock> clusterBlocks = new HashSet<>();
clusterBlocks.add(IndexMetaData.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK);
Map<String, Set<ClusterBlock>> indexLevelBlocks = new HashMap<>();
indexLevelBlocks.put("test", clusterBlocks);
shardFailures.add(new DefaultShardOperationFailedException("test", 0, new ClusterBlockException(indexLevelBlocks)));
BroadcastResponse broadcastResponse = new BroadcastResponse(1, 0, 1, shardFailures);
assertBlocked(broadcastResponse);
}
{
List<DefaultShardOperationFailedException> shardFailures = new ArrayList<>();
Set<ClusterBlock> clusterBlocks = new HashSet<>();
clusterBlocks.add(IndexMetaData.INDEX_READ_BLOCK);
clusterBlocks.add(IndexMetaData.INDEX_METADATA_BLOCK);
Map<String, Set<ClusterBlock>> indexLevelBlocks = new HashMap<>();
indexLevelBlocks.put("test", clusterBlocks);
shardFailures.add(new DefaultShardOperationFailedException("test", 0, new ClusterBlockException(indexLevelBlocks)));
BroadcastResponse broadcastResponse = new BroadcastResponse(1, 0, 1, shardFailures);
assertBlocked(broadcastResponse);
}
{
List<DefaultShardOperationFailedException> shardFailures = new ArrayList<>();
Set<ClusterBlock> clusterBlocks = new HashSet<>();
clusterBlocks.add(IndexMetaData.INDEX_READ_ONLY_BLOCK);
clusterBlocks.add(IndexMetaData.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK);
Map<String, Set<ClusterBlock>> indexLevelBlocks = new HashMap<>();
indexLevelBlocks.put("test", clusterBlocks);
shardFailures.add(new DefaultShardOperationFailedException("test", 0, new ClusterBlockException(indexLevelBlocks)));
BroadcastResponse broadcastResponse = new BroadcastResponse(1, 0, 1, shardFailures);
assertBlocked(broadcastResponse);
}
}
}