Make Snapshot Logic Write Metadata after Segments Backport by mkleen · Pull Request #9327 · crate/crate

mkleen · 2019-11-06T16:01:22Z

Summary of the changes / Why this improves CrateDB

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

Checklist

User relevant changes are recorded in CHANGES.txt
Touched code is covered by tests
Documentation has been updated if necessary
CLA is signed
This does not contain breaking changes, or if it does:
- It is released within a major release
- It is recorded in CHANGES.txt
- It was marked as deprecated in an earlier release if possible
- You've thought about the consequences and other components are adapted
  (E.g. AdminUI)

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

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java

mfussenegger

Could you please also refer to the upstream change within the commit message?

mfussenegger

I think we should also follow up on this with some QA tests (in crate-qa) that test snapshot compatibility between versions and also the mixed-cluster snapshot case.

We could re-use the minio server-layer recently introduced.

es/es-server/src/main/java/org/elasticsearch/repositories/Repository.java

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

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java

seut

looks great, added just few testing related comments.

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java

mfussenegger

Should this have a release notes entry (fix?) and be backported to 4.0?

seut · 2019-11-13T14:05:11Z

Should this have a release notes entry (fix?) and be backported to 4.0?

Yes please.

...s-server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java

mkleen · 2019-11-13T15:40:10Z

Should this have a release notes entry (fix?) and be backported to 4.0?

Yes please.

ok added, maybe have a final look before i merge about the change message.

docs/appendices/release-notes/unreleased.rst

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java

seut

👍

mkleen · 2019-11-14T13:46:21Z

👍

yay

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

mkleen · 2019-11-14T14:11:01Z

retest this please

Running: Test class org.elasticsearch.index.engine.InternalEngineTests

FAILURE: Test testRebuildLocalCheckpointTracker(org.elasticsearch.index.engine.InternalEngineTests)

[2019-11-15T03:04:45,320][INFO ][o.e.i.e.InternalEngineTests] [testRebuildLocalCheckpointTracker] before test

[2019-11-15T03:04:45,436][INFO ][o.e.i.e.InternalEngineTests] [testRebuildLocalCheckpointTracker] after test

REPRODUCE WITH: ./gradlew null -Dtests.seed=18A00E0DB6C5742C -Dtests.class=org.elasticsearch.index.engine.InternalEngineTests -Dtests.method="testRebuildLocalCheckpointTracker" -Dtests.locale=ta-LK -Dtests.timezone=NZ

java.lang.AssertionError: seq_no=54 max_seq_no=62 checkpoint=-1

Expected:

 but: was <false>

This pull request is a backport of elastic/elasticsearch#47283 The purpose of this pull request is to speed up the snapshot finalization. This is archived by parallelizing the writes of the metadata in the snapshot finalization step. Also, this will generally speed up the snapshot process overall in case of large number of indices. This improvement makes sense, because the snapshot finalization takes much longer since #9327 is integrated.

This pull request is a backport of elastic/elasticsearch#47283 The purpose of this pull request is to speed up the snapshot finalization. This is archived by parallelizing the writes of the metadata in the snapshot finalization step. Also, this will generally speed up the snapshot process overall in case of large number of indices. This improvement makes sense, because the snapshot finalization takes much longer since #9327 is integrated. (cherry picked from commit 3091e26)

mkleen force-pushed the mkleen/metadata_after_segment_2 branch 2 times, most recently from 0f56525 to 8b73dac Compare November 6, 2019 16:23

mkleen commented Nov 6, 2019

View reviewed changes

es/es-server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java Outdated Show resolved Hide resolved

mkleen force-pushed the mkleen/metadata_after_segment_2 branch from 8b73dac to f5881f1 Compare November 7, 2019 16:48

mkleen commented Nov 12, 2019

View reviewed changes

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java Outdated Show resolved Hide resolved

mkleen requested a review from mfussenegger November 12, 2019 17:27

mfussenegger reviewed Nov 13, 2019

View reviewed changes

mkleen force-pushed the mkleen/metadata_after_segment_2 branch from 6d23ec3 to aa48a6d Compare November 13, 2019 09:37

mkleen marked this pull request as ready for review November 13, 2019 09:38

mkleen requested a review from seut November 13, 2019 10:05

seut reviewed Nov 13, 2019

View reviewed changes

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java Show resolved Hide resolved

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java Outdated Show resolved Hide resolved

mkleen requested review from mfussenegger and seut November 13, 2019 13:16

mfussenegger reviewed Nov 13, 2019

View reviewed changes

seut reviewed Nov 13, 2019

View reviewed changes

...s-server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java Outdated Show resolved Hide resolved

mkleen force-pushed the mkleen/metadata_after_segment_2 branch 3 times, most recently from 902832c to 9ec4cbf Compare November 13, 2019 16:17

mkleen requested review from mfussenegger and seut November 13, 2019 16:39

seut reviewed Nov 14, 2019

View reviewed changes

docs/appendices/release-notes/unreleased.rst Outdated Show resolved Hide resolved

sql/src/test/java/io/crate/integrationtests/SnapshotRestoreIntegrationTest.java Outdated Show resolved Hide resolved

seut approved these changes Nov 14, 2019

View reviewed changes

Make Snapshot Logic Write Metadata after Segments

d9bb3e7

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

mkleen force-pushed the mkleen/metadata_after_segment_2 branch from 2f7c310 to d9bb3e7 Compare November 14, 2019 13:48

mkleen added v/4.0 ready-to-merge Let Mergify merge the PR once approved and checks pass labels Nov 14, 2019

mergify bot merged commit 60e1600 into master Nov 14, 2019

mergify bot mentioned this pull request Nov 14, 2019

Make Snapshot Logic Write Metadata after Segments Backport (bp #9327) #9349

Merged

mergify bot deleted the mkleen/metadata_after_segment_2 branch November 14, 2019 14:38

mkleen mentioned this pull request Nov 27, 2019

Speed up Snapshot Finalization #9384

Merged

5 tasks

Conversation

mkleen commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the changes / Why this improves CrateDB

Checklist

Uh oh!

Uh oh!

Uh oh!

mfussenegger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfussenegger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seut left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mfussenegger left a comment

Choose a reason for hiding this comment

Uh oh!

seut commented Nov 13, 2019

Uh oh!

Uh oh!

mkleen commented Nov 13, 2019

Uh oh!

Uh oh!

Uh oh!

seut left a comment

Choose a reason for hiding this comment

Uh oh!

mkleen commented Nov 14, 2019

Uh oh!

mkleen commented Nov 14, 2019

FAILURE: Test testRebuildLocalCheckpointTracker(org.elasticsearch.index.engine.InternalEngineTests)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mkleen commented Nov 6, 2019 •

edited

Loading

mfussenegger left a comment •

edited

Loading