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

gtid, relay: use gtid_purged to handle gap in Previous_gtids event #1430

Merged
merged 13 commits into from
Feb 19, 2021

Conversation

lance6716
Copy link
Collaborator

@lance6716 lance6716 commented Feb 5, 2021

What problem does this PR solve?

part of #1418 (we need to test master-slave upstream manually later)

What is changed and how it works?

remove ResetStart, instead, query @@gtid_purged and combine it to gtid set when needed

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has interface methods change

Side effects

  • Potential performance regression

Related changes

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

@lance6716 lance6716 changed the title [WIP] relay: use gtid_purged to handle gap in Previous_gtids event relay: use gtid_purged to handle gap in Previous_gtids event Feb 5, 2021
@lance6716 lance6716 added 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 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Feb 5, 2021
relay/relay_test.go Outdated Show resolved Hide resolved
@lance6716 lance6716 added the status/PTAL This PR is ready for review. Add this label back after committing new changes label Feb 5, 2021
@lance6716 lance6716 changed the title relay: use gtid_purged to handle gap in Previous_gtids event gtid, relay: use gtid_purged to handle gap in Previous_gtids event Feb 5, 2021
// because it doesn't cover all gtid_purged. The error of using it will be
// ERROR 1236 (HY000): The slave is connecting using CHANGE MASTER TO MASTER_AUTO_POSITION = 1, but the master has purged binary logs containing GTIDs that the slave requires.
// so we add gtid_purged to it.
func AddGSetWithPurged(ctx context.Context, gset gtid.Set, conn *sql.Conn) (gtid.Set, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use sql.Conn instead of sql.DB here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

some callers only hold sql.Conn not sql.DB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in line 108 of pkg/v1dbschema/schema.go (below file). I'll close connection in next commit

if err2 != nil {
return err2
}
latestGTID, err2 = utils.AddGSetWithPurged(ctx, latestGTID, dbConn)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't add purged gtid to result.LatestGTIDs, do we need to add purged gtid to latestGTID here?

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

@lance6716 lance6716 added the status/LGT1 One reviewer already commented LGTM label Feb 19, 2021
@lance6716
Copy link
Collaborator Author

/run-all-tests

Copy link
Contributor

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

LGTM

@lance6716 lance6716 merged commit c32de24 into pingcap:master Feb 19, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Feb 19, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #1441

@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 Feb 19, 2021
lance6716 added a commit that referenced this pull request Feb 19, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: lance6716 <lance6716@gmail.com>
@lance6716 lance6716 added this to the v2.0.2 milestone Feb 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated status/LGT1 One reviewer already commented LGTM status/PTAL This PR is ready for review. Add this label back after committing new changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants