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

Mark read-only flush and verify #119743

Merged
merged 17 commits into from
Jan 21, 2025

Conversation

henningandersen
Copy link
Contributor

When marking read-only now flush and mark index as verified. Use this in the deprecation check.

When marking read-only now flush and mark index as verified.
Use this in the deprecation check.
@henningandersen henningandersen added >enhancement :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v9.0.0 labels Jan 8, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @henningandersen, I've created a changelog YAML for you.

@dakrone
Copy link
Member

dakrone commented Jan 13, 2025

One drive-by comment here — we re-use language around calling an index "read-only" and the write block. I assume that we'd be able to do a "verified" write block, and not only a "verified" read_only block, is that correct?

@henningandersen
Copy link
Contributor Author

I assume that we'd be able to do a "verified" write block, and not only a "verified" read_only block, is that correct?

Yes, both are covered, see this line.

There are likely a few corners more to cover in this PR like if both blocks are applied and one is removed, we may want to leave the verified flag intact.

@dakrone
Copy link
Member

dakrone commented Jan 14, 2025

Do you think we'd also be able to do a "verified" block on a closed index? I.e., adding the verification flag to an index that is currently closed.

@henningandersen
Copy link
Contributor Author

A closed index is already flushed and verified, it has its own verified flag at least. I would prefer to just allow closed n-2 indices to be upgraded, though the user will then have to mark them read only before opening on the new version.

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

Looks good, I'm waiting for tests to be added before final approval

@henningandersen henningandersen changed the title POC mark read-only Mark read-only flush and verify Jan 20, 2025
@henningandersen henningandersen marked this pull request as ready for review January 20, 2025 20:22
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Jan 20, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@henningandersen henningandersen requested a review from tlrx January 20, 2025 20:23
@henningandersen henningandersen added auto-backport Automatically create backport pull requests when merged v8.18.0 labels Jan 20, 2025
Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the additionnal test.

@henningandersen henningandersen merged commit 5e8cce2 into elastic:main Jan 21, 2025
16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 119743

tlrx added a commit to tlrx/elasticsearch that referenced this pull request Jan 21, 2025
Now the Add Index Block API automatically synchronizes
the translog, flushes the shard and sets the verified
setting, we can use it in the N-2 upgrade tests instead
of explicit calls.

Relates elastic#119743
Relates elastic#120522
tlrx pushed a commit to tlrx/elasticsearch that referenced this pull request Jan 21, 2025
When marking read-only now flush and mark index as verified guaranteeing
that we can upgrade safely to next version with N-1 indices (becoming N-2).
Use this in the deprecation check.
elasticsearchmachine pushed a commit that referenced this pull request Jan 21, 2025
When marking read-only now flush and mark index as verified guaranteeing
that we can upgrade safely to next version with N-1 indices (becoming N-2).
Use this in the deprecation check.
elasticsearchmachine pushed a commit that referenced this pull request Jan 22, 2025
This change contains follows ups now the Add Index Block API change is
merged.

The index setting `index.verified_read_only`can now be `PrivateIndex`
and not `Dynamic` anymore. Regular indices in version N-2 can recover if
they have the `index.block.read_only` too. And finally, upgrade tests
can use the Add Index Block API instead of manually flushing and adding
blocks explicitly.

This change requires #120537 for `8.x` (tests will fail until it is
merged).

Relates #119743 Relates #120522
@@ -0,0 +1,5 @@
pr: 119743
summary: POC mark read-only
Copy link
Member

@dakrone dakrone Jan 24, 2025

Choose a reason for hiding this comment

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

@henningandersen can we open another PR to make this better for user-facing changelogs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement Team:Distributed Indexing Meta label for Distributed Indexing team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants