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

binlog, relay: support binlog sequence number larger than 999999 #1476

Merged
merged 15 commits into from
Mar 11, 2021
Merged

binlog, relay: support binlog sequence number larger than 999999 #1476

merged 15 commits into from
Mar 11, 2021

Conversation

lance6716
Copy link
Collaborator

@lance6716 lance6716 commented Mar 3, 2021 •
edited
Loading

What problem does this PR solve?

close #1475

What is changed and how it works?

compare by numeric logic, not byte order

Check List

Tests

  • integration test

Code changes

Side effects

Related changes

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

@lance6716
Copy link
Collaborator Author

I'll find a way to do an integration test 😵

@lance6716 lance6716 added this to the v2.0.2 milestone Mar 3, 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 Mar 3, 2021
@lance6716
Copy link
Collaborator Author

need adjust |<upstream switch number> in another PR

@lance6716
Copy link
Collaborator Author

/run-all-tests

@lance6716
Copy link
Collaborator Author

we could inspect docker mount path and modify binlog files. will add integration test later

https://dev.mysql.com/doc/refman/5.7/en/docker-mysql-more-topics.html

@csuzhangxc
Copy link
Member

we could inspect docker mount path and modify binlog files. will add integration test later

https://dev.mysql.com/doc/refman/5.7/en/docker-mysql-more-topics.html

oh, you can even build a special MySQL docker image with pre-generated binlog files.

@lance6716
Copy link
Collaborator Author

/cc @GMHDBJD @lichunzhu

GMHDBJD
GMHDBJD reviewed Mar 10, 2021
View reviewed changes
.github/workflows/binlog-999999.yml Outdated
uses: actions/upload-artifact@v2
if: ${{ always() }}
with:
name: upstream-switch-logs
Copy link
Collaborator

Choose a reason for hiding this comment

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

change name

GMHDBJD
GMHDBJD reviewed Mar 10, 2021
View reviewed changes
go.mod
@@ -49,3 +49,5 @@ require (
)

go 1.13

replace github.com/siddontang/go-mysql v1.1.1-0.20200824131207-0c5789dd0bd3 => github.com/lance6716/go-mysql v1.1.1-0.20210303100354-b0e44c2c5623
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will the pr merged to go-mysql?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/siddontang/go-mysql is not maintained now. maybe we could give a hand after finishing our works 😂

GMHDBJD
GMHDBJD reviewed Mar 10, 2021
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

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

rest LGTM

tests/binlog_999999/docker-compose.yml Outdated
networks:
db-networks:
ipv4_address: 172.28.128.5
image: mysql:5.7.32
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe change to 8.0?

@lichunzhu
Copy link
Contributor

/lgtm

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM label Mar 10, 2021
@lance6716
Copy link
Collaborator Author

ping @GMHDBJD

GMHDBJD
GMHDBJD approved these changes Mar 11, 2021
View reviewed changes
Copy link
Collaborator

@GMHDBJD GMHDBJD 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
Copy link
Collaborator Author

/merge

@ti-srebot
Copy link

@lance6716 Oops! This PR requires at least 2 LGTMs to merge. The current number of LGTM is 1

@lance6716 lance6716 added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Mar 11, 2021
@lance6716
Copy link
Collaborator Author

/merge

@ti-srebot
Copy link

@lance6716 Oops! This PR requires at least 2 LGTMs to merge. The current number of LGTM is 1

@lance6716
Copy link
Collaborator Author

/run-all-tests

@lance6716 lance6716 merged commit fbb6b52 into pingcap:master Mar 11, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Mar 11, 2021
@lance6716 @ti-srebot
cherry pick pingcap#1476 to release-2.0
06f2247 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Mar 11, 2021
Merged
@ti-srebot
Copy link

cherry pick to release-2.0 in PR #1503

@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 Mar 11, 2021
lance6716 pushed a commit that referenced this pull request Mar 12, 2021
@ti-srebot
cherry pick #1476 to release-2.0 (#1503)
f8d64fd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@GMHDBJD GMHDBJD GMHDBJD 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 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated status/LGT2 Two reviewers already commented LGTM, ready for merge
Projects
None yet
Milestone
v2.0.2
Development

Successfully merging this pull request may close these issues.

use numeric comparison when compare binlog filename
5 participants
@lance6716 @csuzhangxc @lichunzhu @ti-srebot @GMHDBJD