Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation #275

ishan16696 · 2020-10-23T04:54:24Z

What this PR does / why we need it:
WALs file can have data revisions which are ahead of data revisions present in DB file, if etcd terminates abnormally and fails to flush the changes from WALs to Bolt DB ,this lead to unnecessary data restoration.
This PR eliminates this unnecessary data restoration. Starting the embedded etcd on data dir, ping the embedded etcd for 60s to get the latest revision from etcd, after that compare this latest revision and latest revision of back-up data.

Which issue(s) this PR fixes:
Fixes #260

Special notes for your reviewer:

Release note:

Validator now double checks latest revision by starting an embedded etcd if DB-based revision check fails. This can potentially avoid unnecessary data restoration when etcd terminates abnormally.

CLAassistant · 2020-10-23T04:54:29Z

All committers have signed the CLA.

amshuman-kr

Thanks a lot for the PR @ishan16696! Just some small suggestions.

Also, do you plan to include the test (copying old data directory and restoring it)?

BTW Could you please sign the CLA?

pkg/initializer/validator/datavalidator.go

amshuman-kr

Thanks for the changes! One small change again :-)

pkg/initializer/validator/datavalidator.go

pkg/initializer/validator/types.go

amshuman-kr

One small change again 😉

pkg/initializer/validator/datavalidator.go

shreyas-s-rao

Thanks @ishan16696 for making the changes. Looks like the change in copyFile function is breaking one of the other test cases. Please look into this and fix it. You can run unit tests locally before pushing changes, by running make verify.

pkg/initializer/validator/types.go

pkg/initializer/validator/datavalidator_test.go

amshuman-kr

LGTM pending unit test results.

pkg/initializer/validator/datavalidator_test.go

…es abruptly, this can lead to unnecessary data restoration.

shreyas-s-rao

/lgtm

gardener-robot added needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Oct 23, 2020

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/ok-to-test-tm Has approval for running integration tests on TestMachinery labels Oct 23, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test-tm Has approval for running integration tests on TestMachinery label Oct 23, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 23, 2020

gardener-robot-ci-3 added the needs/ok-to-test-tm Requires integration tests to be run on TestMachinery label Oct 23, 2020

gardener-robot-ci-1 added the needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 23, 2020

ishan16696 changed the title ~~To avoid Unnecessary Data Restoration #260~~ To avoid Unnecessary Data Restoration Oct 23, 2020

amshuman-kr suggested changes Oct 23, 2020

View reviewed changes

gardener-robot added needs/changes Needs (more) changes and removed needs/review Needs review labels Oct 23, 2020

amshuman-kr changed the title ~~To avoid Unnecessary Data Restoration~~ Double check by starting an embedded etcd if revision check based on data dir fails during data validation Oct 23, 2020

amshuman-kr changed the title ~~Double check by starting an embedded etcd if revision check based on data dir fails during data validation~~ Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation Oct 23, 2020

amshuman-kr requested a review from shreyas-s-rao October 23, 2020 06:33

amshuman-kr suggested changes Oct 23, 2020

View reviewed changes

pkg/initializer/validator/datavalidator.go Outdated Show resolved Hide resolved

pkg/initializer/validator/types.go Outdated Show resolved Hide resolved

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 23, 2020

amshuman-kr suggested changes Oct 23, 2020

View reviewed changes

pkg/initializer/validator/datavalidator.go Outdated Show resolved Hide resolved

gardener-robot-ci-3 removed the reviewed/ok-to-test-tm Has approval for running integration tests on TestMachinery label Oct 23, 2020

ishan16696 force-pushed the master branch from 30470fc to 9c8bbdb Compare November 6, 2020 10:45

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 6, 2020

ishan16696 force-pushed the master branch from 9c8bbdb to 72bce40 Compare November 6, 2020 10:49

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

shreyas-s-rao requested changes Nov 6, 2020

View reviewed changes

pkg/initializer/validator/types.go Outdated Show resolved Hide resolved

pkg/initializer/validator/datavalidator_test.go Outdated Show resolved Hide resolved

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

ishan16696 force-pushed the master branch from 2012bef to cfa45e4 Compare November 6, 2020 12:38

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

amshuman-kr approved these changes Nov 6, 2020

View reviewed changes

amshuman-kr added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 6, 2020

shreyas-s-rao reviewed Nov 8, 2020

View reviewed changes

pkg/initializer/validator/datavalidator_test.go Outdated Show resolved Hide resolved

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 8, 2020

Fixed an issue when WALs file have uncommitted data and etcd terminat…

4526158

…es abruptly, this can lead to unnecessary data restoration.

ishan16696 force-pushed the master branch from 998f222 to 4526158 Compare November 8, 2020 18:59

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 8, 2020

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 8, 2020

shreyas-s-rao approved these changes Nov 9, 2020

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging needs/changes Needs (more) changes and removed needs/changes Needs (more) changes reviewed/lgtm Has approval for merging labels Nov 9, 2020

shreyas-s-rao merged commit 8d50c71 into gardener:master Nov 9, 2020

abdasgupta mentioned this pull request Nov 10, 2020

Added CLI-flags to enable restoration for delta snapshots with large amount of data #282

Merged

ishan16696 mentioned this pull request Jan 9, 2023

[BUG] Etcd failed to flush changes from WAL to the DB before shutting down/crashing. gardener/etcd-druid#99

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation #275

Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation #275

ishan16696 commented Oct 23, 2020 •

edited by shreyas-s-rao

Loading

CLAassistant commented Oct 23, 2020 •

edited

Loading

amshuman-kr left a comment

amshuman-kr left a comment

amshuman-kr left a comment

shreyas-s-rao left a comment

amshuman-kr left a comment

shreyas-s-rao left a comment

Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation #275

Double check latest revision by starting an embedded etcd if revision check based on data dir fails during data validation #275

Conversation

ishan16696 commented Oct 23, 2020 • edited by shreyas-s-rao Loading

CLAassistant commented Oct 23, 2020 • edited Loading

amshuman-kr left a comment

Choose a reason for hiding this comment

amshuman-kr left a comment

Choose a reason for hiding this comment

amshuman-kr left a comment

Choose a reason for hiding this comment

shreyas-s-rao left a comment

Choose a reason for hiding this comment

amshuman-kr left a comment

Choose a reason for hiding this comment

shreyas-s-rao left a comment

Choose a reason for hiding this comment

ishan16696 commented Oct 23, 2020 •

edited by shreyas-s-rao

Loading

CLAassistant commented Oct 23, 2020 •

edited

Loading