Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

restore: fix error lost in create schema #530

Merged
merged 8 commits into from
Dec 23, 2020
Merged

restore: fix error lost in create schema #530

merged 8 commits into from
Dec 23, 2020

Conversation

3pointer
Copy link
Contributor

@3pointer 3pointer commented Dec 22, 2020

What problem does this PR solve?

Fix pingcap/tidb#21827

What is changed and how it works?

break create loop immediately when error encountered

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to cherry-pick to the release branch

Release note:

  • Fix the issue that lost create table error during multiple tables created.

for tbl := range tablesSchema {
tables = append(tables, tbl)
}
// sort tables for failpoint test
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a better solution is to use regexp to match the execute result in failpoint test

)
failpoint.Inject("sqlCreateStmts", func() {
if i == 0 {
Copy link
Contributor

@glorv glorv Dec 23, 2020

Choose a reason for hiding this comment

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

if the count matters, I think you can add an int parameter to this fail-point to determine at which condition, it should return an error. So maybe we can test with the error returns at first, middle or last position.

lightning/restore/restore.go Outdated Show resolved Hide resolved
lightning/restore/tidb.go Outdated Show resolved Hide resolved
lightning/restore/restore.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Dec 23, 2020
@glorv
Copy link
Contributor

glorv commented Dec 23, 2020

LGTM

@ti-srebot ti-srebot added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/LGT1 One reviewer already commented LGTM (LGTM1) labels Dec 23, 2020
@glorv
Copy link
Contributor

glorv commented Dec 23, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8510040 into master Dec 23, 2020
@kennytm kennytm deleted the lost_err branch December 23, 2020 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CanMerge status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) type/bug-fix Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tidb-lighting does not log schema creation errors
4 participants