Forbid the removal of the write block if the index is read-only #120648

tlrx · 2025-01-22T16:47:07Z

Ensure that a write block cannot be removed on a read-only compatible index in version N-2, while allowing to change a read_only block into a write block if needed as well as closing/reopening such indices.

Requires #120647 to be merged on 8.x.

Relates ES-10320

elasticsearchmachine · 2025-01-22T16:48:07Z

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

henningandersen

Looks good though I hope we can support removing one of two write blocks too? See comment.

henningandersen · 2025-01-22T22:11:52Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java

                    if (blocks.hasIndexBlock(index, block)) {
                        blocks.removeIndexBlock(index, block);
                        changed = true;
                        if (block.contains(ClusterBlockLevel.WRITE)) {


I think this is too harsh. In case the index has both the read-only and the write block, it should be allowed to remove either.

In particular, if the user applied read-only on 8.x, upgraded and then want to update some settings, they should be allowed to add the write block and remove the read-only block.

I recognize this is also a flaw in the original code in that it removes the verified setting.

I could be ok to let this go in if we follow-up with the additional detail. But would prefer we do it in one go if it is not too hard.

I think this is too harsh. In case the index has both the read-only and the write block, it should be allowed to remove either.

I agree, it makes sense.

In particular, if the user applied read-only on 8.x, upgraded and then want to update some settings, they should be allowed to add the write block and remove the read-only block.

I initially thought that updating both settings at once would not work because of the code allowing setting updates when there is a METADATA_WRITE block in place:

elasticsearch/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java

Lines 87 to 91 in 45ae071

if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock an index

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;

but it seems that request.settings().size() == 1 does not always apply since the great setting refactoring here.

It looks like this refactoring introduced a change in the parenthesis impacting the conditions to bypass the check. Which will serve us today.

tlrx · 2025-01-27T10:03:22Z

Ok, so it took me a couple of days to get this working with the subtleties of blocks updates. Thanks for your patience on this @henningandersen.

I updated this PR (and #120647) to forbid the removal of the ClusterBlockLevel.WRITE on a read-only compatible index if at least one node of the cluster cannot write to it (see verifyReadOnlyIndices).

Since both index.blocks.read_only to index.blocks.write contains this ClusterBlockLevel.WRITE we can allow updating the index blocks setting to move from one type of block to the other. In the tests I included the use case you mentioned: updating from index.blocks.read_only to index.blocks.write.

I also changed the MetadataUpdateSettingsService#maybeUpdateClusterBlock so that the VERIFIED_READ_ONLY_SETTING remains as long as a ClusterBlockLevel.WRITE remains (required for blocks settings updates). It also allows to keep the VERIFIED_READ_ONLY_SETTING value when closing a verified read-only index, because closed indices have the INDEX_CLOSED_BLOCK that also contains the ClusterBlockLevel.WRITE block. When reopening, the INDEX_CLOSED_BLOCK is removed, removing the VERIFIED_READ_ONLY_SETTING if no other write block is in place and it will fail the reopening since the check in verifyReadOnlyIndices does not allow a read-only compatible index to have no ClusterBlockLevel.WRITE block (applying a write block before reopening will work).

I largely updated the integration tests to to have a bit more randomization around blocks and closing/opening during upgrades.

Note that there is still a bit missing for fully supporting closed indices (allowing a verified-before-close index to be upgraded without being marked as read-only first): I'll do this in follow ups.

henningandersen

LGTM.

henningandersen · 2025-01-27T15:25:49Z

server/src/main/java/org/elasticsearch/cluster/metadata/MetadataUpdateSettingsService.java

+    private static void verifyReadOnlyIndices(@Nullable Set<Index> readOnlyIndices, ClusterBlocks blocks) {
+        if (readOnlyIndices != null) {
+            for (Index readOnlyIndex : readOnlyIndices) {
+                if (blocks.indexBlocked(ClusterBlockLevel.WRITE, readOnlyIndex.getName()) == false) {


Should this use the new hasIndexBlockLevel method to avoid interaction from global blocks?

Right, I pushed de6a0bd.

henningandersen · 2025-01-27T15:56:03Z

...vaRestTest/java/org/elasticsearch/lucene/RollingUpgradeLuceneIndexCompatibilityTestCase.java

                closeIndex(restoredIndex);
                ensureGreen(restoredIndex);

+                logger.debug("--> write API block can be removed on a closed index: INDEX_CLOSED_BLOCK already blocks writes");


Can we also check that if an index has both read-only and write block then we can remove one of them for an N-2 index?

I reworked the testIndexUpgrade test to randomly add one or two of the blocks, and then remove one of them.

See 59e7acd

…ly (#120647) Forbids on 8.x the removal/update of a write block if at least one node of the cluster cannot write the index. Backport of #120648 for 8.18. Relates ES-10320

tlrx · 2025-01-28T15:40:11Z

Thanks Henning

Forbid the removal of the write block if the index is read-only

d4fcf56

tlrx added >non-issue :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v9.0.0 labels Jan 22, 2025

elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jan 22, 2025

tlrx requested a review from henningandersen January 22, 2025 17:54

henningandersen reviewed Jan 22, 2025

View reviewed changes

tlrx added 4 commits January 23, 2025 13:48

Merge branch 'main' into 2025/01/22/forbid-write-block-removal

38882e0

Merge branch 'main' into 2025/01/22/forbid-write-block-removal

82db1f8

rework block & settings handling

930c705

Merge branch 'main' into 2025/01/22/forbid-write-block-removal

87b5e46

tlrx mentioned this pull request Jan 27, 2025

[8.x] Forbid the removal of the write block if the index is read-only #120647

Merged

tlrx requested a review from henningandersen January 27, 2025 13:52

henningandersen approved these changes Jan 27, 2025

View reviewed changes

Merge branch 'main' into 2025/01/22/forbid-write-block-removal

8e2798a

tlrx added 3 commits January 28, 2025 14:20

feedback

de6a0bd

feedback

59e7acd

Merge branch 'main' into 2025/01/22/forbid-write-block-removal

49eb183

tlrx merged commit 0569fc7 into elastic:main Jan 28, 2025
16 checks passed

tlrx deleted the 2025/01/22/forbid-write-block-removal branch January 28, 2025 15:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Forbid the removal of the write block if the index is read-only #120648

Forbid the removal of the write block if the index is read-only #120648

Uh oh!

tlrx commented Jan 22, 2025 •

edited

Loading

Uh oh!

elasticsearchmachine commented Jan 22, 2025

Uh oh!

henningandersen left a comment

Uh oh!

henningandersen Jan 22, 2025

Uh oh!

tlrx Jan 23, 2025

Uh oh!

tlrx commented Jan 27, 2025

Uh oh!

henningandersen left a comment

Uh oh!

henningandersen Jan 27, 2025

Uh oh!

tlrx Jan 28, 2025

Uh oh!

henningandersen Jan 27, 2025

Uh oh!

tlrx Jan 28, 2025

Uh oh!

Uh oh!

tlrx commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock an index
	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;

Forbid the removal of the write block if the index is read-only #120648

Forbid the removal of the write block if the index is read-only #120648

Uh oh!

Conversation

tlrx commented Jan 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 22, 2025

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

tlrx Jan 23, 2025

Choose a reason for hiding this comment

Uh oh!

tlrx commented Jan 27, 2025

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

tlrx Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

henningandersen Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

tlrx Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tlrx commented Jan 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tlrx commented Jan 22, 2025 •

edited

Loading