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

feat: add handle-error command #850

Merged
merged 46 commits into from
Aug 13, 2020
Merged

Conversation

GMHDBJD
Copy link
Collaborator

@GMHDBJD GMHDBJD commented Jul 31, 2020

What problem does this PR solve?

  • add handle-error command to skip/replace binlog event

What is changed and how it works?

  • When a event error, we print the startLocation and endLocation for the event. So user can use handle --binlog-pos startLocation skip or change the checkpoint with endLocation to skip event.
  • add suffix in Location to distinguish the replace event
    // GetEvent return a replace binlog event
    // for example:
    //			startLocation		endLocation
    // event 1		1000, 0			1010, 0
    // event 2		1010, 0			1020, 0	<--replace it with event a,b,c
    // replace event a	1010, 0			1010, 1
    // replace event b	1010, 1			1010, 2
    // replace event c	1010, 2			1020, 0
    // event 3		1020, 0			1030, 0
    
  • remove sql-skip/replace/inject

Check List

Tests

  • Unit test
  • Integration test

@GMHDBJD GMHDBJD added priority/important Major change, requires approval from ≥2 primary reviewers needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated status/WIP This PR is still work in progress labels Jul 31, 2020
dm/master/server.go Outdated Show resolved Hide resolved
dm/worker/subtask.go Show resolved Hide resolved
Copy link
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

please update PR's description to help me understand 🤣

pkg/binlog/position.go Show resolved Hide resolved
pkg/binlog/position_test.go Outdated Show resolved Hide resolved
syncer/syncer.go Outdated Show resolved Hide resolved
@GMHDBJD
Copy link
Collaborator Author

GMHDBJD commented Aug 5, 2020

/run-all-tests

"github.com/pingcap/parser"
"github.com/pingcap/parser/ast"
tmysql "github.com/pingcap/parser/mysql"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

I found in other code we don't need to split 3-rd package from pingcap and others.

@lance6716
Copy link
Collaborator

LGTM

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM label Aug 12, 2020
@lance6716 lance6716 removed the status/PTAL This PR is ready for review. Add this label back after committing new changes label Aug 12, 2020
_utils/terror_gen/errors_release.txt Show resolved Hide resolved
syncer/syncer.go Show resolved Hide resolved
syncer/syncer.go Outdated Show resolved Hide resolved
@GMHDBJD
Copy link
Collaborator Author

GMHDBJD commented Aug 13, 2020

/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. cooooooool

@csuzhangxc csuzhangxc added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Aug 13, 2020
@GMHDBJD GMHDBJD merged commit a58d2d3 into pingcap:master Aug 13, 2020
@csuzhangxc
Copy link
Member

docs PR pingcap/docs-dm#227

@csuzhangxc csuzhangxc added already-update-docs The docs related to this PR already updated. Add this label once the docs are updated and removed needs-update-docs Should update docs after this PR is merged. Remove this label once the docs are updated labels Aug 14, 2020
@csuzhangxc csuzhangxc removed the needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated label Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
already-update-docs The docs related to this PR already updated. Add this label once the docs are updated priority/important Major change, requires approval from ≥2 primary reviewers status/LGT2 Two reviewers already commented LGTM, ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants