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

Make Snapshot Logic Write Metadata after Segments #45689

Merged
merged 12 commits into from Aug 30, 2019

Conversation

original-brownbear
Copy link
Member

@original-brownbear original-brownbear commented Aug 18, 2019

* WIP, this still needs a docs fixup
* Fixes elastic#41581
* Fixes elastic#25281
@original-brownbear original-brownbear added >bug WIP :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.4.0 labels Aug 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@original-brownbear original-brownbear marked this pull request as ready for review August 19, 2019 05:39
@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/1
(unrelated ML failure)

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/1
(unrelated Grok failure)

@@ -3856,76 +3811,6 @@ public void testParallelRestoreOperationsFromSingleSnapshot() throws Exception {
assertThat(client.prepareGet(restoredIndexName2, typeName, sameSourceIndex ? docId : docId2).get().isExists(), equalTo(true));
}

public void testAbortedSnapshotDuringInitDoesNotStart() throws Exception {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test doesn't really apply anymore with snapshot initialization gone.

@@ -276,9 +276,11 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize, b
// We do some checks in case there is a consistent state for a blob to prevent turning it inconsistent.
final boolean hasConsistentContent =
relevantActions.size() == 1 && relevantActions.get(0).operation == Operation.PUT;
if (BlobStoreRepository.INDEX_LATEST_BLOB.equals(blobName)) {
if (BlobStoreRepository.INDEX_LATEST_BLOB.equals(blobName)
|| blobName.startsWith(BlobStoreRepository.METADATA_PREFIX)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

On master fail-over, we can now have overwriting for meta- blobs here. Since any metadata will be at least as fresh as all the uploaded segments overwrites here should be.

Copy link
Contributor

@andrershov andrershov left a comment

Choose a reason for hiding this comment

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

@original-brownbear I left a couple of comments.

@@ -2538,28 +2536,15 @@ public void testCloseOrDeleteIndexDuringSnapshot() throws Exception {

Client client = client();

boolean allowPartial = randomBoolean();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is that changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't block on the initialize part of the snapshot anymore, so I removed the code paths in this test that required blocking on initialize. allowPartial == true required blocking on init => all the paths that had it set to true were removed.

@original-brownbear
Copy link
Member Author

Jenkins test this
(some weirdness when resolving deps)

@original-brownbear
Copy link
Member Author

Thanks @andrershov ! All fixed I think.

@original-brownbear
Copy link
Member Author

Jenkins run elasticsearch-ci/2

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Sep 30, 2019
As a result of elastic#45689 snapshot finalization started to
take significantly longer than before. This may be a
little unfortunate since it increases the likelihood
of failing to finalize after having written out all
the segment blobs.
This change parallelizes all the metadata writes that
can safely run in parallel in the finalization step to
speed the finalization step up again. Also, this will
generally speed up the snapshot process overall in case
of large number of indices.

This is also a nice to have for elastic#46250 since we add yet
another step (deleting of old index- blobs in the shards
to the finalization.
original-brownbear added a commit that referenced this pull request Sep 30, 2019
As a result of #45689 snapshot finalization started to
take significantly longer than before. This may be a
little unfortunate since it increases the likelihood
of failing to finalize after having written out all
the segment blobs.
This change parallelizes all the metadata writes that
can safely run in parallel in the finalization step to
speed the finalization step up again. Also, this will
generally speed up the snapshot process overall in case
of large number of indices.

This is also a nice to have for #46250 since we add yet
another step (deleting of old index- blobs in the shards
to the finalization.
mkleen added a commit to crate/crate that referenced this pull request Nov 13, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.
mkleen added a commit to crate/crate that referenced this pull request Nov 13, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.
mkleen added a commit to crate/crate that referenced this pull request Nov 14, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.
mergify bot pushed a commit to crate/crate that referenced this pull request Nov 14, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.
mergify bot pushed a commit to crate/crate that referenced this pull request Nov 14, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.

(cherry picked from commit 60e1600)

# Conflicts:
#	docs/appendices/release-notes/unreleased.rst
mkleen added a commit to crate/crate that referenced this pull request Nov 14, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.

(cherry picked from commit 60e1600)
mkleen added a commit to crate/crate that referenced this pull request Nov 19, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.

(cherry picked from commit 60e1600)
seut pushed a commit to crate/crate that referenced this pull request Nov 20, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.

(cherry picked from commit 60e1600)
mergify bot pushed a commit to crate/crate that referenced this pull request Nov 20, 2019
Backport of elastic/elasticsearch#45689
Write metadata during snapshot finalization after segment files
to prevent outdated metadata in case of dynamic mapping updates.

(cherry picked from commit 60e1600)
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Dec 14, 2019
With elastic#45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Closes elastic#50200
original-brownbear added a commit that referenced this pull request Dec 16, 2019
With #45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates #50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Dec 16, 2019
With elastic#45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates elastic#50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Dec 16, 2019
With elastic#45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates elastic#50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
original-brownbear added a commit that referenced this pull request Dec 16, 2019
With #45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates #50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
original-brownbear added a commit that referenced this pull request Dec 16, 2019
* Fix Index Deletion during Snapshot Finalization (#50202)

With #45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates #50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
With elastic#45689 making it so that index metadata is written
after all shards have been snapshotted we can't delete indices
that are part of the upcoming snapshot finalization any longer
and it is not sufficient to check if all shards of an index have been
snapshotted before deciding that it is safe to delete it.
This change forbids deleting any index that is in the process of being
snapshot to avoid issues during snapshot finalization.

Relates elastic#50200 (doesn't fully fix yet because we're not fixing the `partial=true`
snapshot case here
@original-brownbear original-brownbear restored the 41581-fix branch August 6, 2020 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v7.5.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshots should write out IndexMetaData after shard data
5 participants