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

*: support recover relay log file with a subset of metadata #530

Merged
merged 4 commits into from
Mar 12, 2020
Merged

*: support recover relay log file with a subset of metadata #530

merged 4 commits into from
Mar 12, 2020

Conversation

csuzhangxc
Copy link
Member

@csuzhangxc csuzhangxc commented Mar 10, 2020 •
edited
Loading

What problem does this PR solve?

fix #443 , this should be release under v1.0.4.

What is changed and how it works?

  • support recover relay log file with a subset of metadata

    • check whether the metadata is a subset of the relay log file

  • also update go-mysql to fix GTIDSet.Equal.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported variable/fields change
  • Has persistent data change

Side effects

  • Increased code complexity

Related changes

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

@csuzhangxc csuzhangxc added priority/normal Minor change, requires approval from ≥1 primary reviewer status/WIP This PR is still work in progress type/bug-fix Bug fix needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Mar 10, 2020
@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

@csuzhangxc csuzhangxc added status/PTAL This PR is ready for review. Add this label back after committing new changes and removed status/WIP This PR is still work in progress labels Mar 11, 2020
@csuzhangxc
Copy link
Member Author

@WangXiangUSTC @lichunzhu PTAL

@csuzhangxc csuzhangxc added this to the v1.0.4 milestone Mar 11, 2020
WangXiangUSTC
WangXiangUSTC reviewed Mar 12, 2020
View reviewed changes
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

do we need to add kill -9 dm-worker in integration test?
rest LGTM

pkg/binlog/event/common.go
@@ -50,7 +50,7 @@ func GenCommonFileHeader(flavor string, serverID uint32, gSet gtid.Set) ([]*repl
Flags: defaultHeaderFlags,
}
latestPos = uint32(len(replication.BinLogFileHeader))
prevGTIDsEv *replication.BinlogEvent // for MySQL, this will be a GenericEvent
prevGTIDsEv *replication.BinlogEvent
Copy link
Contributor

Choose a reason for hiding this comment

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

is the new go-mysql change the rule？

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, new go-mysql adds a new PreviousGTIDsEvent struct to replace GenericEvent with EventType=PREVIOUS_GTIDS_EVENT.

@csuzhangxc
Copy link
Member Author

do we need to add kill -9 dm-worker in integration test?

@WangXiangUSTC like a special type of chaos test? In fact, this is a litter hard to simulate it by simply kill the process (when just writing the data).

@WangXiangUSTC
Copy link
Contributor

do we need to add kill -9 dm-worker in integration test?

@WangXiangUSTC like a special type of chaos test? In fact, this is a litter hard to simulate it by simply kill the process (when just writing the data).

ok , we can ignore it now

@WangXiangUSTC
Copy link
Contributor

LGTM

@WangXiangUSTC WangXiangUSTC added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Mar 12, 2020
lichunzhu
lichunzhu reviewed Mar 12, 2020
View reviewed changes
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.

Rest LGTM

pkg/terror/error_list.go Outdated
@@ -642,7 +642,7 @@ var (
ErrBinlogWriterWriteDataLen = New(codeBinlogWriterWriteDataLen, ClassFunctional, ScopeInternal, LevelHigh, "data length %d")
ErrBinlogWriterFileNotOpened = New(codeBinlogWriterFileNotOpened, ClassFunctional, ScopeInternal, LevelHigh, "file %s not opened")
ErrBinlogWriterFileSync = New(codeBinlogWriterFileSync, ClassFunctional, ScopeInternal, LevelHigh, "sync file")
ErrBinlogPrevGTIDEvNotValid = New(codeBinlogPrevGTIDEvNotValid, ClassFunctional, ScopeInternal, LevelHigh, "PreviousGTIDsEvent should be a GenericEvent in go-mysql, but got %T")
ErrBinlogPrevGTIDEvNotValid = New(codeBinlogPrevGTIDEvNotValid, ClassFunctional, ScopeInternal, LevelHigh, "PreviousGTIDsEvent should be a PreviousGTIDsEvent in go-mysql, but got %T")
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't understand this error message.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated in 08679dd.

@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

@lichunzhu lichunzhu added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Mar 12, 2020
@codecov
Copy link

codecov bot commented Mar 12, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@d3b0152). Click here to learn what that means.
The diff coverage is 83.3333%.

@@            Coverage Diff             @@
##             master      #530   +/-   ##
==========================================
  Coverage          ?   56.831%           
==========================================
  Files             ?       183           
  Lines             ?     18921           
  Branches          ?         0           
==========================================
  Hits              ?     10753           
  Misses            ?      7080           
  Partials          ?      1088

@csuzhangxc csuzhangxc merged commit 291439b into pingcap:master Mar 12, 2020
@csuzhangxc csuzhangxc deleted the fix-relay-truncate branch March 12, 2020 10:09
@sre-bot sre-bot mentioned this pull request Mar 12, 2020
Closed
@sre-bot
Copy link

sre-bot commented Mar 12, 2020

cherry pick to release-1.0 in PR #531

@sre-bot sre-bot added the already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked label Mar 12, 2020
@sre-bot sre-bot removed the needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 label Mar 12, 2020
csuzhangxc added a commit to csuzhangxc/dm that referenced this pull request Mar 12, 2020
@csuzhangxc
*: support recover relay log file with a subset of metadata (pingcap#530
394fb45 
)

(cherry picked from commit 291439b)
@csuzhangxc csuzhangxc mentioned this pull request Mar 12, 2020
Merged
csuzhangxc added a commit that referenced this pull request Mar 12, 2020
@csuzhangxc
*: support recover relay log file with a subset of metadata (#530) (#534
e27bf8c 
)
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
@csuzhangxc
*: support recover relay log file with a subset of metadata (pingcap#530
9802a12 
)
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
@csuzhangxc @lichunzhu
*: support recover relay log file with a subset of metadata (pingcap#530
f7da6b1 
)
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 7, 2020
@csuzhangxc @lichunzhu
*: support recover relay log file with a subset of metadata (pingcap#530
fd19e12 
)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

@lichunzhu lichunzhu lichunzhu approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.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 type/bug-fix Bug fix
Projects
None yet
Milestone
v1.0.4
Development

Successfully merging this pull request may close these issues.

fial to truncate GTIDs if relay.meta out of date before killed when recovering
4 participants
@csuzhangxc @WangXiangUSTC @sre-bot @lichunzhu