From acf031cdb5f8292730a6c697a8f267523c83ae08 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 1 Jul 2020 12:57:34 +0100 Subject: [PATCH] Forbid read-only-allow-delete block in blocks API (#58727) * Forbid read-only-allow-delete block in blocks API The read-only-allow-delete block is not really under the user's control since Elasticsearch adds/removes it automatically. This commit removes support for it from the new API for adding blocks to indices that was introduced in #58094. * Missing xref * Reword paragraph on read-only-allow-delete block --- docs/reference/index-modules/blocks.asciidoc | 35 +++++++++------- .../modules/cluster/disk_allocator.asciidoc | 2 + .../rest-api-spec/api/indices.add_block.json | 2 +- .../elasticsearch/blocks/SimpleBlocksIT.java | 41 ++++++++++++++----- .../readonly/AddIndexBlockRequest.java | 3 ++ .../elasticsearch/test/ESIntegTestCase.java | 3 +- 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/docs/reference/index-modules/blocks.asciidoc b/docs/reference/index-modules/blocks.asciidoc index 3f66991d0687d..b9cc29740ea1c 100644 --- a/docs/reference/index-modules/blocks.asciidoc +++ b/docs/reference/index-modules/blocks.asciidoc @@ -22,16 +22,6 @@ index: Set to `true` to make the index and index metadata read only, `false` to allow writes and metadata changes. -`index.blocks.read_only_allow_delete`:: - - Similar to `index.blocks.read_only`, but also allows deleting the index to - make more resources available. The <> may add and remove this block automatically. - -Deleting documents from an index to release resources - rather than deleting the index itself - can increase the index size over time. When `index.blocks.read_only_allow_delete` is set to `true`, deleting documents is not permitted. However, deleting the index itself releases the read-only index block and makes resources available almost immediately. - -IMPORTANT: {es} adds and removes the read-only index block automatically when the disk utilization falls below the high watermark, controlled by <>. - `index.blocks.read`:: Set to `true` to disable read operations against the index. @@ -46,6 +36,26 @@ IMPORTANT: {es} adds and removes the read-only index block automatically when th Set to `true` to disable index metadata reads and writes. +`index.blocks.read_only_allow_delete`:: + + Similar to `index.blocks.read_only`, but also allows deleting the index to + make more resources available. The <> adds and removes this block automatically. + +Deleting documents from an index - rather than deleting the index itself - can +in fact increase the index size. When you are running out of disk space +`index.blocks.read_only_allow_delete` is set to `true`, preventing you from +consuming more disk space by deleting some documents. However, this block does +permit you to delete the index itself since this does not require any extra +disk space. When you delete an index the data is removed from disk almost +immediately, freeing the space it consumes. + +IMPORTANT: {es} adds the read-only-allow-delete index block automatically when +disk utilisation exceeds the <> and removes it again when disk utilisation is below the +<>. You should not apply this +block yourself. + [discrete] [[add-index-block]] === Add index block API @@ -94,11 +104,6 @@ Disable read operations. `read_only`:: Disable write operations and metadata changes. -`read_only_allow_delete`:: -Disable write operations and metadata changes. -Document deletion is disabled. -However, index deletion is still allowed. - `write`:: Disable write operations. However, metadata changes are still allowed. ==== diff --git a/docs/reference/modules/cluster/disk_allocator.asciidoc b/docs/reference/modules/cluster/disk_allocator.asciidoc index 0c90df41e2578..fdc93ab75ede5 100644 --- a/docs/reference/modules/cluster/disk_allocator.asciidoc +++ b/docs/reference/modules/cluster/disk_allocator.asciidoc @@ -13,6 +13,7 @@ file or updated dynamically on a live cluster with the Defaults to `true`. Set to `false` to disable the disk allocation decider. +[[cluster-routing-watermark-low]] `cluster.routing.allocation.disk.watermark.low`:: Controls the low watermark for disk usage. It defaults to `85%`, meaning @@ -22,6 +23,7 @@ file or updated dynamically on a live cluster with the amount of space is available. This setting has no effect on the primary shards of newly-created indices but will prevent their replicas from being allocated. +[[cluster-routing-watermark-high]] `cluster.routing.allocation.disk.watermark.high`:: Controls the high watermark. It defaults to `90%`, meaning that diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.add_block.json b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.add_block.json index 797e53bc2a2a7..e2b22ed93ef5f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/indices.add_block.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/indices.add_block.json @@ -19,7 +19,7 @@ }, "block":{ "type":"string", - "description":"The block to add (one of read, write, read_only, metadata, read_only_allow_delete)" + "description":"The block to add (one of read, write, read_only or metadata)" } } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java b/server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java index 1700a78ea7098..01a7eb56bd687 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/blocks/SimpleBlocksIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.indices.create.CreateIndexResponse; +import org.elasticsearch.action.admin.indices.readonly.AddIndexBlockRequestBuilder; import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequestBuilder; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; @@ -177,6 +178,9 @@ public void testAddBlocksWhileExistingBlocks() { ensureGreen("test"); for (APIBlock otherBlock : APIBlock.values()) { + if (otherBlock == APIBlock.READ_ONLY_ALLOW_DELETE) { + continue; + } for (APIBlock block : Arrays.asList(APIBlock.READ, APIBlock.WRITE)) { try { @@ -216,20 +220,20 @@ public void testAddBlocksWhileExistingBlocks() { public void testAddBlockToMissingIndex() { IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().admin().indices() - .prepareAddBlock(randomFrom(APIBlock.values()),"test").get()); + .prepareAddBlock(randomAddableBlock(), "test").get()); assertThat(e.getMessage(), is("no such index [test]")); } public void testAddBlockToOneMissingIndex() { createIndex("test1"); final IndexNotFoundException e = expectThrows(IndexNotFoundException.class, - () -> client().admin().indices().prepareAddBlock(randomFrom(APIBlock.values()),"test1", "test2").get()); + () -> client().admin().indices().prepareAddBlock(randomAddableBlock(), "test1", "test2").get()); assertThat(e.getMessage(), is("no such index [test2]")); } public void testCloseOneMissingIndexIgnoreMissing() throws Exception { createIndex("test1"); - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); try { assertBusy(() -> assertAcked(client().admin().indices().prepareAddBlock(block, "test1", "test2") .setIndicesOptions(lenientExpandOpen()))); @@ -241,13 +245,20 @@ public void testCloseOneMissingIndexIgnoreMissing() throws Exception { public void testAddBlockNoIndex() { final ActionRequestValidationException e = expectThrows(ActionRequestValidationException.class, - () -> client().admin().indices().prepareAddBlock(randomFrom(APIBlock.values())).get()); + () -> client().admin().indices().prepareAddBlock(randomAddableBlock()).get()); assertThat(e.getMessage(), containsString("index is missing")); } public void testAddBlockNullIndex() { expectThrows(NullPointerException.class, - () -> client().admin().indices().prepareAddBlock(randomFrom(APIBlock.values()), (String[])null)); + () -> client().admin().indices().prepareAddBlock(randomAddableBlock(), (String[])null)); + } + + public void testCannotAddReadOnlyAllowDeleteBlock() { + createIndex("test1"); + final AddIndexBlockRequestBuilder request = client().admin().indices().prepareAddBlock(APIBlock.READ_ONLY_ALLOW_DELETE, "test1"); + final ActionRequestValidationException e = expectThrows(ActionRequestValidationException.class, request::get); + assertThat(e.getMessage(), containsString("read_only_allow_delete block is for internal use only")); } public void testAddIndexBlock() throws Exception { @@ -258,7 +269,7 @@ public void testAddIndexBlock() throws Exception { indexRandom(randomBoolean(), false, randomBoolean(), IntStream.range(0, nbDocs) .mapToObj(i -> client().prepareIndex(indexName).setId(String.valueOf(i)).setSource("num", i)).collect(toList())); - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); try { assertAcked(client().admin().indices().prepareAddBlock(block, indexName)); assertIndexHasBlock(block, indexName); @@ -278,7 +289,7 @@ public void testSameBlockTwice() throws Exception { indexRandom(randomBoolean(), false, randomBoolean(), IntStream.range(0, randomIntBetween(1, 10)) .mapToObj(i -> client().prepareIndex(indexName).setId(String.valueOf(i)).setSource("num", i)).collect(toList())); } - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); try { assertAcked(client().admin().indices().prepareAddBlock(block, indexName)); assertIndexHasBlock(block, indexName); @@ -300,7 +311,7 @@ public void testAddBlockToUnassignedIndex() throws Exception { assertThat(clusterState.metadata().indices().get(indexName).getState(), is(IndexMetadata.State.OPEN)); assertThat(clusterState.routingTable().allShards().stream().allMatch(ShardRouting::unassigned), is(true)); - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); try { assertAcked(client().admin().indices().prepareAddBlock(block, indexName)); assertIndexHasBlock(block, indexName); @@ -321,7 +332,7 @@ public void testConcurrentAddBlock() throws InterruptedException { final CountDownLatch startClosing = new CountDownLatch(1); final Thread[] threads = new Thread[randomIntBetween(2, 5)]; - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); try { for (int i = 0; i < threads.length; i++) { @@ -356,7 +367,7 @@ public void testAddBlockWhileIndexingDocuments() throws Exception { final String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT); createIndex(indexName); - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); int nbDocs = 0; @@ -400,7 +411,7 @@ public void testAddBlockWhileDeletingIndices() throws Exception { final List threads = new ArrayList<>(); final CountDownLatch latch = new CountDownLatch(1); - final APIBlock block = randomFrom(APIBlock.values()); + final APIBlock block = randomAddableBlock(); Consumer exceptionConsumer = t -> { Throwable cause = ExceptionsHelper.unwrapCause(t); @@ -478,4 +489,12 @@ static void assertIndexHasBlock(APIBlock block, final String... indices) { public static void disableIndexBlock(String index, APIBlock block) { disableIndexBlock(index, block.settingName()); } + + /** + * The read-only-allow-delete block cannot be added via the add index block API; this method chooses randomly from the values that + * the add index block API does support. + */ + private static APIBlock randomAddableBlock() { + return randomValueOtherThan(APIBlock.READ_ONLY_ALLOW_DELETE, () -> randomFrom(APIBlock.values())); + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockRequest.java index 72e8707f969ed..10d3241946736 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/readonly/AddIndexBlockRequest.java @@ -63,6 +63,9 @@ public ActionRequestValidationException validate() { if (CollectionUtils.isEmpty(indices)) { validationException = addValidationError("index is missing", validationException); } + if (block == APIBlock.READ_ONLY_ALLOW_DELETE) { + validationException = addValidationError("read_only_allow_delete block is for internal use only", validationException); + } return validationException; } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 6de472e082e32..2561e69243b41 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1470,7 +1470,8 @@ public static void disableIndexBlock(String index, String block) { /** Enables an index block for the specified index */ public static void enableIndexBlock(String index, String block) { - if (randomBoolean()) { + if (IndexMetadata.APIBlock.fromSetting(block) == IndexMetadata.APIBlock.READ_ONLY_ALLOW_DELETE || randomBoolean()) { + // the read-only-allow-delete block isn't supported by the add block API so we must use the update settings API here. Settings settings = Settings.builder().put(block, true).build(); client().admin().indices().prepareUpdateSettings(index).setSettings(settings).get(); } else {