Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix affect of online DDL #466

Merged
merged 12 commits into from
Feb 27, 2019
Merged

Fix affect of online DDL #466

merged 12 commits into from
Feb 27, 2019

Conversation

july2993
Copy link
Contributor

@july2993 july2993 commented Feb 15, 2019

What problem does this PR solve?

see https://internal.pingcap.net/jira/browse/TOOL-906

should update drainer if upgrade TiDB to version with pingcap/tidb#9207 merged

What is changed and how it works?

skip the truncated table DML data.
refactor some translator code in ./drainer

add some ddl test case

change tests/run.sh to be easy start multi instance TiDB for test
currently only handle the truncate table case, other DDL should take no effect.

Check List

Tests

  • Integration Test

Code changes

Related changes

  • Need to cherry-pick to the release branch

@IANTHEREAL
Copy link
Collaborator

respect need more test, you can take your thought into action now @july2993

@july2993
Copy link
Contributor Author

respect need more test, you can take your thought into action now @july2993

has add the Integration test for other ddl case test, no integration test when write need more test

drainer/schema.go Outdated Show resolved Hide resolved
Co-Authored-By: july2993 <july2993@gmail.com>
drainer/syncer.go Outdated Show resolved Hide resolved
tests/dailytest/ddl.go Show resolved Hide resolved
tests/dailytest/ddl.go Outdated Show resolved Hide resolved
tests/run.sh Outdated Show resolved Hide resolved
@IANTHEREAL
Copy link
Collaborator

Rest LGTM

@july2993
Copy link
Contributor Author

@GregoryIan @WangXiangUSTC PTAL

@july2993
Copy link
Contributor Author

/rebuild

@july2993
Copy link
Contributor Author

/run-all-tests

@IANTHEREAL
Copy link
Collaborator

LGTM

// TiDB write DDL Binlog for every DDL Job, we must ignore jobs that are cancelled or rollback
// For older version TiDB, it write DDL Binlog in the txn that the state of job is changed to *synced*
// Now, it write DDL Binlog in the txn that the state of job is changed to *done* (before change to *synced*)
// At state *done*, it will be always and only changed to *synced*.
func skipJob(job *model.Job) bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WangXiangUSTC @GregoryIan
if upgrade to TiDB with the write ddl binlog when change state to done, must upgrade drainer too, or may there's risk losing ddl event(skip here because it's still at done state)

Copy link
Collaborator

Choose a reason for hiding this comment

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

send an email to business team @july2993

return types.NewDatum(col.DefaultValue)
}

if col.Tp == mysql.TypeEnum {
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC Feb 26, 2019

Choose a reason for hiding this comment

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

seems table.GetZeroValue have already handle the mysql.TypeEnum.
Or you need handle the mysql.TypeSet too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in GetZeroValue will get a zero value for enum, this is not a valid value for Enum, but for set
a empty set is a valid value.this is some kind like https://github.com/pingcap/tidb/blob/9da174902af4005d7d3951d09cd5b9cd648d392c/table/column.go#L388
also you can take a look at https://dev.mysql.com/doc/refman/5.7/en/data-type-defaults.html

@WangXiangUSTC
Copy link
Contributor

LGTM @crazycs520 PTAL

@july2993 july2993 merged commit fb3949e into master Feb 27, 2019
@july2993 july2993 deleted the hjh/fix_ddl branch February 27, 2019 03:30
july2993 added a commit that referenced this pull request Feb 27, 2019
see issue pingcap/tidb#9304

skip the truncated table DML data.
refactor some translator code in ./drainer
add some ddl test case
change tests/run.sh to be easy start multi instance TiDB for test
july2993 added a commit that referenced this pull request Feb 27, 2019
see issue pingcap/tidb#9304

skip the truncated table DML data.
refactor some translator code in ./drainer
add some ddl test case
change tests/run.sh to be easy start multi instance TiDB for test
july2993 added a commit that referenced this pull request Feb 28, 2019
see issue pingcap/tidb#9304

skip the truncated table DML data.
refactor some translator code in ./drainer
add some ddl test case
change tests/run.sh to be easy start multi instance TiDB for test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants