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

ddl: save checkpoint with schemaVersion #1033

Merged
merged 11 commits into from
Jan 11, 2021

Conversation

3pointer
Copy link
Contributor

@3pointer 3pointer commented Dec 31, 2020

What problem does this PR solve?

Fix #1032

What is changed and how it works?

save schemaVersion to checkpoint.
with amend txn enabled. prewrite.SchemeVersion could out of date.
so when drainer restart, we need to compare checkpoint'SchemaVersion and prewrite.SchemaVersion.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

Release note

  • Fix the issue that when amend txn enabled. Drainer chooses the wrong schema version of DDL to generate the wrong SQL.

@3pointer 3pointer changed the title ddl: save checkpoint with schemaVersion WIP: ddl: save checkpoint with schemaVersion Dec 31, 2020
@sre-bot
Copy link

sre-bot commented Dec 31, 2020

@3pointer 3pointer changed the title WIP: ddl: save checkpoint with schemaVersion ddl: save checkpoint with schemaVersion Jan 7, 2021
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -92,6 +93,7 @@ func (sp *FileCheckPoint) Save(ts, secondaryTS int64, consistent bool) error {

sp.CommitTS = ts
sp.ConsistentSaved = consistent
sp.Version = version
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we prevent the version being reduced here?

Copy link
Contributor

Choose a reason for hiding this comment

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

A recently test case shows that drainer writes the old schema version again after manually adjusting the schema version to the correct value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why?

Copy link
Contributor

Choose a reason for hiding this comment

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

The test case with amend transaction enabled.

dml ddl schema version
begin 1
insert...
ddls from 1 to 10
commit 10

The ddls are executed before transaction committed and push the schema version to 10 in checkpoint. When the drainer is executing the dml transaction, the schema version of it is still 1, and will checkpoint with schema version 1 into checkpoint.

eventCounter.WithLabelValues("savepoint").Add(1)
}

log.Info("handleSuccess quit")
}

func (s *Syncer) savePoint(ts, secondaryTS int64) {
func (s *Syncer) savePoint(ts, secondaryTS, version int64) {
if ts < s.cp.TS() {
log.Error("save ts is less than checkpoint ts %d", zap.Int64("save ts", ts), zap.Int64("checkpoint ts", s.cp.TS()))
}

log.Info("write save point", zap.Int64("ts", ts))
Copy link
Contributor

Choose a reason for hiding this comment

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

May also log version in this function.

drainer/syncer.go Outdated Show resolved Hide resolved
Copy link
Contributor

@you06 you06 left a comment

Choose a reason for hiding this comment

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

LGTM

lichunzhu
lichunzhu previously approved these changes Jan 11, 2021
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

LGTM

@3pointer
Copy link
Contributor Author

/run-all-tests

csuzhangxc
csuzhangxc previously approved these changes Jan 11, 2021
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@3pointer 3pointer dismissed stale reviews from csuzhangxc and lichunzhu via 04e144a January 11, 2021 08:21
@3pointer
Copy link
Contributor Author

/run-all-tests

@3pointer
Copy link
Contributor Author

/run-all-tests

Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@3pointer
Copy link
Contributor Author

/run-all-tests

@3pointer 3pointer merged commit e28b75c into pingcap:master Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unexpected Unknown column error in downstream SQL execution
5 participants