Make Snapshot Logic Write Metadata after Segments #45689

original-brownbear · 2019-08-18T20:37:18Z

Write metadata during snapshot finalization after segment files to prevent outdated metadata in case of dynamic mapping updates as explained in Snapshots should write out IndexMetaData after shard data #41581
Keep the old behavior of writing the metadata beforehand in the case of mixed version clusters for BwC reasons
- Still overwrite the metadata in the end, so even a mixed version cluster is fixed by this change if a newer version master does the finalization
Fixes Snapshots should write out IndexMetaData after shard data #41581

* WIP, this still needs a docs fixup * Fixes elastic#41581 * Fixes elastic#25281

elasticmachine · 2019-08-18T20:37:19Z

Pinging @elastic/es-distributed

server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java

original-brownbear · 2019-08-19T07:49:42Z

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

original-brownbear · 2019-08-19T08:38:43Z

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

original-brownbear · 2019-08-19T09:12:57Z

server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

@@ -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 {


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

original-brownbear · 2019-08-19T09:14:55Z

.../src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java

@@ -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)) {


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.

andrershov

@original-brownbear I left a couple of comments.

server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

andrershov · 2019-08-26T15:23:59Z

server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java

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

        Client client = client();

-        boolean allowPartial = randomBoolean();


Why is that changed?

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.

server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java

original-brownbear · 2019-08-26T17:42:38Z

Jenkins test this
(some weirdness when resolving deps)

original-brownbear · 2019-08-26T17:54:01Z

Thanks @andrershov ! All fixed I think.

original-brownbear · 2019-08-26T20:24:02Z

Jenkins run elasticsearch-ci/2

…o 41581-fix

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.

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.

Backport of elastic/elasticsearch#45689 Write metadata during snapshot finalization after segment files to prevent outdated metadata in case of dynamic mapping updates.

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

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)

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

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

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

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

* 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

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

Make Snapshot Logic Write Metadata after Segments

51100e2

* WIP, this still needs a docs fixup * Fixes elastic#41581 * Fixes elastic#25281

original-brownbear added >bug WIP :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.4.0 labels Aug 18, 2019

original-brownbear added 2 commits August 19, 2019 05:27

fix test

78a18f4

introduce BwC logic and fix documentation

16c6213

original-brownbear removed the WIP label Aug 19, 2019

original-brownbear marked this pull request as ready for review August 19, 2019 05:39

original-brownbear commented Aug 19, 2019

View reviewed changes

server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java Outdated Show resolved Hide resolved

original-brownbear added 4 commits August 19, 2019 07:50

fix empty line change and use constant

f2e4272

nicer looking test

d1b32c5

Merge remote-tracking branch 'elastic/master' into 41581-fix

62930d0

adjust failing test

70b6379

original-brownbear requested review from andrershov and tlrx August 19, 2019 09:11

original-brownbear commented Aug 19, 2019

View reviewed changes

andrershov suggested changes Aug 26, 2019

View reviewed changes

original-brownbear added 2 commits August 26, 2019 19:23

Merge remote-tracking branch 'elastic/master' into 41581-fix

ba503c6

CR: comments

f844f8a

original-brownbear requested a review from andrershov August 26, 2019 17:54

Merge branch 'master' of https://github.com/elastic/elasticsearch int…

4b67146

…o 41581-fix

original-brownbear mentioned this pull request Sep 30, 2019

Speed up Snapshot Finalization (#47283) #47309

Merged

This was referenced Nov 4, 2019

Make Snapshot Logic Write Metadata after Segments Backport crate/crate#9317

Closed

Make Snapshot Logic Write Metadata after Segments Backport crate/crate#9327

Merged

original-brownbear mentioned this pull request Dec 14, 2019

Fix Index Deletion during Snapshot Finalization #50202

Merged

original-brownbear mentioned this pull request Dec 16, 2019

Fix Index Deletion during Snapshot Finalization (#50202) #50227

Merged

original-brownbear mentioned this pull request Dec 16, 2019

Fix Index Deletion during Snapshot Finalization (#50202) #50228

Merged

This was referenced Feb 3, 2020

[meta] 7.6 release elastic/elasticsearch-net#4340

Closed

[meta] 7.6 release elastic/elasticsearch-net#4341

Closed

original-brownbear mentioned this pull request Mar 30, 2020

Remove Redundant Cleanup Step in Snapshot Failure #54395

Merged

original-brownbear restored the 41581-fix branch August 6, 2020 18:35

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Snapshot Logic Write Metadata after Segments #45689

Make Snapshot Logic Write Metadata after Segments #45689

original-brownbear commented Aug 18, 2019 •

edited

Loading

elasticmachine commented Aug 18, 2019

original-brownbear commented Aug 19, 2019

original-brownbear commented Aug 19, 2019

original-brownbear Aug 19, 2019

original-brownbear Aug 19, 2019

andrershov left a comment

andrershov Aug 26, 2019

original-brownbear Aug 26, 2019

original-brownbear commented Aug 26, 2019

original-brownbear commented Aug 26, 2019

original-brownbear commented Aug 26, 2019

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

		Client client = client();

		boolean allowPartial = randomBoolean();

Make Snapshot Logic Write Metadata after Segments #45689

Make Snapshot Logic Write Metadata after Segments #45689

Conversation

original-brownbear commented Aug 18, 2019 • edited Loading

elasticmachine commented Aug 18, 2019

original-brownbear commented Aug 19, 2019

original-brownbear commented Aug 19, 2019

original-brownbear Aug 19, 2019

Choose a reason for hiding this comment

original-brownbear Aug 19, 2019

Choose a reason for hiding this comment

andrershov left a comment

Choose a reason for hiding this comment

andrershov Aug 26, 2019

Choose a reason for hiding this comment

original-brownbear Aug 26, 2019

Choose a reason for hiding this comment

original-brownbear commented Aug 26, 2019

original-brownbear commented Aug 26, 2019

original-brownbear commented Aug 26, 2019

original-brownbear commented Aug 18, 2019 •

edited

Loading