Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Jan 22, 2025

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 tlrx added >non-issue :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. backport v8.18.0 labels Jan 22, 2025
@tlrx tlrx requested a review from henningandersen January 22, 2025 17:54
if (blocks.hasIndexBlock(index, block)) {
blocks.removeIndexBlock(index, block);
changed = true;
if (block.contains(ClusterBlockLevel.WRITE)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In relation to my comment on the v9 PR, just noting that this should follow whatever we do on that PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the behavior in the V9 PR and here, see this comment

@tlrx tlrx force-pushed the 2025/01/22/forbid-write-block-removal-8.x branch from b418880 to 7d57b16 Compare January 24, 2025 17:07
@tlrx tlrx force-pushed the 2025/01/22/forbid-write-block-removal-8.x branch from 114ae6e to e84ed85 Compare January 27, 2025 07:51
@tlrx tlrx requested a review from henningandersen January 27, 2025 10:04
@tlrx
Copy link
Member Author

tlrx commented Jan 27, 2025

This is ready for another review, the behavior has been updated and is described in this comment.

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the 9.0 PR, I think we should use the new hasIndexBlockLevel here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I pushed 66325bf (use hasIndexBlockLevel at the ClusterBlocks level, while the other is at the Builder level).

@tlrx
Copy link
Member Author

tlrx commented Jan 28, 2025

Build failure is similar to #120980

@tlrx tlrx merged commit 76a4a05 into elastic:8.x Jan 28, 2025
15 checks passed
@tlrx tlrx deleted the 2025/01/22/forbid-write-block-removal-8.x branch January 28, 2025 12:55
@tlrx
Copy link
Member Author

tlrx commented Jan 28, 2025

Thanks Henning!

tlrx added a commit that referenced this pull request Jan 28, 2025
)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. >non-issue v8.18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants