Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

source: check whether GTID is ON in the upstream ealier #946

Merged
merged 3 commits into from
Aug 29, 2020
Merged

source: check whether GTID is ON in the upstream ealier #946

merged 3 commits into from
Aug 29, 2020

Conversation

GMHDBJD
Copy link
Collaborator

@GMHDBJD GMHDBJD commented Aug 28, 2020

What problem does this PR solve?

close #941

What is changed and how it works?

check whether GTID is ON in the upstream when operate-source create

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    run mysql with GTID_MODE=OFF, and set enable-gtid: true in source config

@GMHDBJD GMHDBJD added priority/normal Minor change, requires approval from ≥1 primary reviewer status/PTAL This PR is ready for review. Add this label back after committing new changes type/bug-fix Bug fix needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 labels Aug 28, 2020
lance6716
lance6716 reviewed Aug 28, 2020
View reviewed changes
dm/config/source_config.go Outdated
@@ -318,3 +318,18 @@ func (c *SourceConfig) check(metaData *toml.MetaData, err error) error {
c.adjust()
return nil
}

// PreCheck check valify source config
func (c *SourceConfig) PreCheck(db *sql.DB) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adjust could check and return error, so maybe we could put these code into Adjust?

Copy link
Member

Choose a reason for hiding this comment

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

agree. seems we have use *sql.DB in Adjust now.

@lance6716
Copy link
Collaborator

LGTM

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM label Aug 28, 2020
csuzhangxc
csuzhangxc approved these changes Aug 29, 2020
View reviewed changes
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

@csuzhangxc csuzhangxc added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Aug 29, 2020
@csuzhangxc csuzhangxc added this to the v2.0.0 RC.2 milestone Aug 29, 2020
@csuzhangxc csuzhangxc merged commit 824e6b0 into pingcap:master Aug 29, 2020
@ti-srebot ti-srebot mentioned this pull request Aug 29, 2020
Merged
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #957

@ti-srebot ti-srebot added already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 labels Aug 29, 2020
lance6716 pushed a commit that referenced this pull request Aug 29, 2020
@ti-srebot
source: check whether GTID is ON in the upstream ealier (#946) (#957)
7f7a501
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lance6716 lance6716 lance6716 left review comments

@csuzhangxc csuzhangxc csuzhangxc approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge status/PTAL This PR is ready for review. Add this label back after committing new changes type/bug-fix Bug fix
Projects
None yet
Milestone
v2.0.0 RC.2
Development

Successfully merging this pull request may close these issues.

check whether GTID is ON in the upstream ealier
4 participants
@GMHDBJD @lance6716 @ti-srebot @csuzhangxc