Skip to content

Mark read-only flush and verify#119743

Merged
henningandersen merged 17 commits intoelastic:mainfrom
henningandersen:poc_mark_read_only
Jan 21, 2025
Merged

Mark read-only flush and verify#119743
henningandersen merged 17 commits intoelastic:mainfrom
henningandersen:poc_mark_read_only

Conversation

@henningandersen
Copy link
Contributor

@henningandersen henningandersen commented Jan 8, 2025

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

elasticsearchmachine commented Jan 8, 2025

Hi @henningandersen, I've created a changelog YAML for you.
@henningandersen henningandersen mentioned this pull request Jan 13, 2025
Closed
@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

henningandersen commented Jan 14, 2025

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

henningandersen commented Jan 15, 2025

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.
tlrx
tlrx reviewed Jan 20, 2025
View reviewed changes
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 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 (obsolete) Meta label for Distributed Indexing team. Obsolete. Please do not use. label Jan 20, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 20, 2025

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
tlrx
tlrx approved these changes Jan 21, 2025
View reviewed changes
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

elasticsearchmachine commented Jan 21, 2025

💔 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
@henningandersen henningandersen mentioned this pull request Jan 21, 2025
Merged
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Jan 21, 2025
@tlrx
Use Add Index Block API in N-2 upgrade tests
4187d1c 
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 tlrx mentioned this pull request Jan 21, 2025
Merged
tlrx pushed a commit to tlrx/elasticsearch that referenced this pull request Jan 21, 2025
@henningandersen
Mark read-only flush and verify (elastic#119743)
1f26512 
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
@henningandersen
Mark read-only flush and verify (#119743) (#120522)
c5a623a 
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
@tlrx
Follow ups after Add Index Block API changes for N-2 support (#120526)
93bc758 
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
dakrone
dakrone reviewed Jan 24, 2025
View reviewed changes
docs/changelog/119743.yaml
@@ -0,0 +1,5 @@
pr: 119743
summary: POC mark read-only
Copy link
Member

@dakrone dakrone Jan 24, 2025 •
edited
Loading

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?
@pxsalehi pxsalehi mentioned this pull request Feb 18, 2025
Closed
@dakrone dakrone mentioned this pull request Mar 13, 2025
Merged
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Mar 13, 2025
@dakrone
Fix changelog to make more sense for release notes
9f539e5 
Relates to elastic#119743
dakrone added a commit that referenced this pull request Mar 13, 2025
@dakrone
Fix changelog to make more sense for release notes (#124774)
b49b22d 
Relates to #119743
@kingherc kingherc mentioned this pull request Jul 16, 2025
Closed
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/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement Team:Distributed Indexing (obsolete) Meta label for Distributed Indexing team. Obsolete. Please do not use. v8.18.0 v9.0.0

4 participants

@henningandersen @elasticsearchmachine @dakrone @tlrx