Use table info by 3pointer · Pull Request #231 · pingcap/br

3pointer · 2020-04-08T09:28:00Z

What problem does this PR solve?

Fix #230 (comment)

What is changed and how it works?

use ddljob.TableInfo and ddljob.DBInfo to create tables and schema in incremental restore.
set config.AlterPrimaryKey to true so we can execute drop/add pk DDL in incremental restore.

Check List

Tests

Integration test
Manual test (add detailed scripts or steps below)

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

Need to cherry-pick to the release branch

3pointer · 2020-04-08T09:34:05Z

/run-all-tests

3pointer · 2020-04-08T09:45:12Z

/run-all-tests

pkg/restore/db.go

kennytm · 2020-04-08T09:48:38Z

tests/br_incompatible_tidb_config/run.sh

 run_br --pd $PD_ADDR backup db --db "$DB" -s "local://$TEST_DIR/$DB$TABLE"

+run_sql "create table $DB.$INCREMENTAL_TABLE (a int primary key, b int unique);"
+run_sql "insert into $DB.$INCREMENTAL_TABLE values (42, 42);"


please include a test case for CREATE TABLE … LIKE.

okay, add test case below

3pointer · 2020-04-08T12:01:49Z

/run-integration-test

3pointer · 2020-04-08T12:36:53Z

/run-integration-test

3pointer · 2020-04-08T13:11:53Z

/run-integration-test

3pointer · 2020-04-09T03:29:36Z

/run-integration-test

codecov · 2020-04-09T05:31:10Z

Codecov Report

Merging #231 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #231   +/-   ##
=======================================
  Coverage   70.31%   70.31%           
=======================================
  Files          48       48           
  Lines        4743     4743           
=======================================
  Hits         3335     3335           
  Misses        966      966           
  Partials      442      442

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 634701b...ff642ba. Read the comment docs.

tests/br_incompatible_tidb_config/run.sh

3pointer · 2020-04-10T13:14:56Z

/run-integration-test

overvenus

LGTM

3pointer · 2020-04-11T03:36:34Z

run-integration-test

3pointer · 2020-04-11T03:36:46Z

/run-integration-test

* create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt

* enable tidb config by default (#230) * enable tidb config by default * backup,restore: fix --checksum flag. (#223) * backup,restore: work on progress to fix a bug that causes --checksum flag won't work properly. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: backup will report total bytes and kvs when checksums check disabled. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: backup will report total bytes and kvs when checksums check disabled. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: add log to ChecksumMatches and new version of FastChecksum. Some of log has been lose. They are in ChecksumMatches now. * restore: restore could find non-checksum tables and skip them automatically. for backup, ChecksumMatches returns error now. * misc: add document for Table::NoChecksum. * backup: omit checksum progress bar when user specify `--checksum=false`. * backup: `CopyMetaFrom` overrides original `client.Schemes` instead of append at its end. * backup: refactor about checksum logic, fix a bug. the bug would cause: when multi tables are backup, the metadata contains only one table. * backup: do some lints. * backup,restore: do some refactor so that cyclomatic complexity won't be too large. * misc: don't use underscore on receiver. * backup: print "quick checksum success" message per table. ...to make br_full_index happy! * backup: refactor a MinInt pattern. * backup: Apply suggestions from code review Co-Authored-By: kennytm <kennytm@gmail.com> Co-authored-by: 3pointer <luancheng@pingcap.com> Co-authored-by: kennytm <kennytm@gmail.com> * fill size in SSTMeta (#233) * Use table info (#231) * create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt * update 3.1 dependency * remove conf.Experimental.AllowsExpressionIndex Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com> Co-authored-by: kennytm <kennytm@gmail.com>

* create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt

* enable tidb config by default (#230) * enable tidb config by default * backup,restore: fix --checksum flag. (#223) * backup,restore: work on progress to fix a bug that causes --checksum flag won't work properly. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: backup will report total bytes and kvs when checksums check disabled. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: backup will report total bytes and kvs when checksums check disabled. Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure. * backup: add log to ChecksumMatches and new version of FastChecksum. Some of log has been lose. They are in ChecksumMatches now. * restore: restore could find non-checksum tables and skip them automatically. for backup, ChecksumMatches returns error now. * misc: add document for Table::NoChecksum. * backup: omit checksum progress bar when user specify `--checksum=false`. * backup: `CopyMetaFrom` overrides original `client.Schemes` instead of append at its end. * backup: refactor about checksum logic, fix a bug. the bug would cause: when multi tables are backup, the metadata contains only one table. * backup: do some lints. * backup,restore: do some refactor so that cyclomatic complexity won't be too large. * misc: don't use underscore on receiver. * backup: print "quick checksum success" message per table. ...to make br_full_index happy! * backup: refactor a MinInt pattern. * backup: Apply suggestions from code review Co-Authored-By: kennytm <kennytm@gmail.com> Co-authored-by: 3pointer <luancheng@pingcap.com> Co-authored-by: kennytm <kennytm@gmail.com> * Use table info (#231) * create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt * backup: generate backupmeta when backup empty. (#235) * cmd: don't use ':' in the default log file name (#236) * Update build status badge (#239) * pass sse_kms_key_id to S3 (#243) * redirect kvproto Signed-off-by: Yi Wu <yiwu@pingcap.com> * pass sse_kms_key_id Signed-off-by: Yi Wu <yiwu@pingcap.com> * fix hound Signed-off-by: Yi Wu <yiwu@pingcap.com> * update kvproto Signed-off-by: Yi Wu <yiwu@pingcap.com> * go mod tidy Signed-off-by: Yi Wu <yiwu@pingcap.com> Co-authored-by: kennytm <kennytm@gmail.com> * storage: support placing the S3/GCS options into the storage URL (#246) * update 4.0 dependency * go mod tidy * Update README.md Co-Authored-By: Neil Shen <overvenus@gmail.com> Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com> Co-authored-by: kennytm <kennytm@gmail.com> Co-authored-by: Neil Shen <overvenus@gmail.com> Co-authored-by: yiwu-arbug <yiwu@pingcap.com>

3pointer added 2 commits April 8, 2020 17:10

create with info during incremental restore

fe4343f

add test

7e7feed

3pointer added the WIP label Apr 8, 2020

fix log

87f841c

kennytm reviewed Apr 8, 2020

View reviewed changes

address comment

cc98e8b

fix ci

f976e41

3pointer added type/feature-request New feature or request and removed WIP labels Apr 9, 2020

kennytm reviewed Apr 10, 2020

View reviewed changes

tests/br_incompatible_tidb_config/run.sh Show resolved Hide resolved

3pointer added 2 commits April 10, 2020 18:22

address commemnt

6b13f1d

Merge branch 'master' into use_table_info

85bab03

overvenus approved these changes Apr 11, 2020

View reviewed changes

Merge branch 'master' into use_table_info

ff642ba

kennytm approved these changes Apr 11, 2020

View reviewed changes

kennytm merged commit 84e0bbc into pingcap:master Apr 11, 2020

3pointer deleted the use_table_info branch April 13, 2020 02:05

3pointer added a commit to 3pointer/br that referenced this pull request Apr 13, 2020

Use table info (pingcap#231)

8af84a4

* create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt

3pointer added a commit to 3pointer/br that referenced this pull request Apr 26, 2020

Use table info (pingcap#231)

2899256

* create with info during incremental restore * add test * fix log * address comment * fix ci * address commemnt

Conversation

3pointer commented Apr 8, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

3pointer commented Apr 8, 2020

Uh oh!

3pointer commented Apr 8, 2020

Uh oh!

Uh oh!

kennytm Apr 8, 2020

Choose a reason for hiding this comment

Uh oh!

3pointer Apr 10, 2020

Choose a reason for hiding this comment

Uh oh!

3pointer commented Apr 8, 2020

Uh oh!

3pointer commented Apr 8, 2020

Uh oh!

3pointer commented Apr 8, 2020

Uh oh!

3pointer commented Apr 9, 2020

Uh oh!

codecov bot commented Apr 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

3pointer commented Apr 10, 2020

Uh oh!

overvenus left a comment

Choose a reason for hiding this comment

Uh oh!

3pointer commented Apr 11, 2020

Uh oh!

3pointer commented Apr 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Apr 9, 2020 •

edited

Loading