*: add `Writer` for relay log #117

csuzhangxc · 2019-04-18T09:03:13Z

What problem does this PR solve?

this PR is a part of #91.

What is changed and how it works?

add a FileWriter to support to write binlog events into files
- write a binlog file header for any new binlog file
- ignore any fake RotateEvent
- ignore any duplicate events
- fill any potential holes in the binlog file with dummy events
The FileWriter support to recover any binlog file with uncompleted events/transactions

Check List

Tests

Unit test

Code changes

Has exported function/method change
Has exported variable/fields change
Has interface methods change

Side effects

Increased code complexity

relay/writer/file_test.go

Co-Authored-By: csuzhangxc <[email protected]>

…nctions

csuzhangxc · 2019-04-22T06:37:48Z

/run-all-tests

pkg/binlog/filename.go

csuzhangxc · 2019-05-13T07:13:08Z

/run-all-tests

csuzhangxc · 2019-05-13T07:24:18Z

/run-all-tests

codecov · 2019-05-13T07:30:10Z

Codecov Report

Merging #117 into master will increase coverage by 1.7257%.
The diff coverage is 82.2429%.

@@               Coverage Diff                @@
##             master       #117        +/-   ##
================================================
+ Coverage   32.4824%   34.2082%   +1.7257%     
================================================
  Files           117        121         +4     
  Lines         13124      13602       +478     
================================================
+ Hits           4263       4653       +390     
- Misses         8378       8420        +42     
- Partials        483        529        +46

IANTHEREAL · 2019-05-13T13:09:26Z

relay/writer/file.go

+// handleDuplicateEventsExist tries to handle a potential duplicate event in the binlog file.
+func (w *FileWriter) handleDuplicateEventsExist(ev *replication.BinlogEvent) (*Result, error) {
+	filename := filepath.Join(w.cfg.RelayDir, w.filename.Get())
+	duplicate, err := checkIsDuplicateEvent(filename, ev)


is it inefficient that call it for every event?

in most cases, checkIsDuplicateEvent only checks start/end pos with the file size (has a comment in checkIsDuplicateEvent), so I think it is not inefficient. and if we want to make it work correctly, we must do this check.

I'm worrid that os.Stat is an inefficient function

so, pass file size into the function as an arg?

for os.Stat, it will call syscall.Stat; for File.Stat, it will call syscall.Fstat.

from http://man7.org/linux/man-pages/man2/stat.2.html.

fstat() is identical to stat(), except that the file about which
information is to be retrieved is specified by the file descriptor
fd.

if we changed to pass File.Stat() into the function, the difference of efficiency is small? or should we need to pass an in-memory offset into the function? @GregoryIan what's your opinion.

I think we can choose one

check duplicate event only if we need to check, like holeSize < 0

or parse a fd or stat information as arg?

I choose the first one.

pkg/binlog/writer/file.go

IANTHEREAL · 2019-05-16T04:29:05Z

/run-all-tests

amyangfei · 2019-05-16T05:01:41Z

/run-all-tests

# Conflicts: # pkg/binlog/reader/tcp.go

csuzhangxc · 2019-05-20T09:33:05Z

/run-all-tests

IANTHEREAL · 2019-05-21T06:31:06Z

relay/writer/file_util.go

+
+	// only parse single event
+	eof, err := replication.NewBinlogParser().ParseSingleEvent(f, onEventFunc)
+	if err != nil {


if some events after format description event are damaged, but found == true, should we return true?

I changed to return true even err != nil in 6d54140, because this function only checks if FormatDescriptionEvent exists.

…e < 0

…matDescriptionEventExist`

IANTHEREAL · 2019-05-21T07:27:44Z

Rest LGTM

IANTHEREAL

LGTM

relay/writer/file.go

csuzhangxc added 2 commits April 16, 2019 19:41

write: add a binlog file writer

5ae5bec

writer: basic structure

98252cc

csuzhangxc added priority/important Major change, requires approval from ≥2 primary reviewers status/WIP This PR is still work in progress type/enhancement Performance improvement or refactoring labels Apr 18, 2019

writer: add test case for starting underlying writer

1b54643

kennytm reviewed Apr 18, 2019

View reviewed changes

relay/writer/file_test.go Outdated Show resolved Hide resolved

kennytm and others added 9 commits April 19, 2019 10:20

Update relay/writer/file_test.go

10767be

Co-Authored-By: csuzhangxc <[email protected]>

*: address comments

6246da8

writer: check binlog file header and FormatDescriptionEvent helper fu…

0516ef0

…nctions

*: handle FormatDescriptionEvent in FileWriter

d24c04c

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

b4633ef

writer: fix CI

79f7262

writer: handle RotateEvent

f89ea7c

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

f2940f6

writer: add test for RotateEvent with FormatDescriptionEvent

4ef41f1

csuzhangxc added 13 commits April 22, 2019 17:08

event: change GenPreviousGTIDsEvent to return a GenericEvent

6a28944

writer: add test case with DDL and DML

2b62d66

writer: update offset after events wrote

d6b2d1e

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

50f4187

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

595f276

event: GenDummyEvent (USER_VAR_EVENT/QueryEvent)

8d8f289

writer: use dummy event to fill the potential hole

18f1367

writer: check event is duplicate

ed3015c

writer: handle duplicate events

298e87c

writer: close FileReader in defer

ea41ba8

*: refine code structure

866df1e

event: get GTID set from a PreviousGTIDsEvent

5c640fc

event: get GTID set from a MariaDBGTIDListEvent

b071768

*: address comments

63d1b7b

amyangfei reviewed May 13, 2019

View reviewed changes

pkg/binlog/filename.go Outdated Show resolved Hide resolved

csuzhangxc added 2 commits May 13, 2019 14:57

writer: address comments

67393f7

writer: address comments

a4a3b15

IANTHEREAL reviewed May 13, 2019

View reviewed changes

amyangfei reviewed May 14, 2019

View reviewed changes

pkg/binlog/writer/file.go Show resolved Hide resolved

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

96e108e

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

0d3de7d

# Conflicts: # pkg/binlog/reader/tcp.go

IANTHEREAL reviewed May 21, 2019

View reviewed changes

csuzhangxc added 3 commits May 21, 2019 14:33

writer: address comment, handleDuplicateEventsExist only when holeSiz…

80fd184

…e < 0

writer: refine Writer interface

d536bd4

writer: return true when FormatDescriptionEvent exist for `checkFor…

6d54140

…matDescriptionEventExist`

writer: check hole and duplicate for RotateEvent

16e0574

IANTHEREAL reviewed May 21, 2019

View reviewed changes

Merge remote-tracking branch 'remotes/origin/master' into relay-writer-2

6802a07

amyangfei reviewed May 22, 2019

View reviewed changes

relay/writer/file.go Show resolved Hide resolved

writer: address comment

2f332b0

amyangfei approved these changes May 22, 2019

View reviewed changes

amyangfei added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels May 22, 2019

csuzhangxc merged commit 7152fa0 into pingcap:master May 22, 2019

csuzhangxc deleted the relay-writer-2 branch May 22, 2019 02:34

lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020

*: add Writer for relay log (pingcap#117)

d771f5f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add `Writer` for relay log #117

*: add `Writer` for relay log #117

csuzhangxc commented Apr 18, 2019 •

edited

Loading

csuzhangxc commented Apr 22, 2019

csuzhangxc commented May 13, 2019

csuzhangxc commented May 13, 2019

codecov bot commented May 13, 2019 •

edited

Loading

IANTHEREAL May 13, 2019

csuzhangxc May 13, 2019

IANTHEREAL May 21, 2019

csuzhangxc May 21, 2019

csuzhangxc May 21, 2019

IANTHEREAL May 21, 2019

csuzhangxc May 21, 2019

IANTHEREAL commented May 16, 2019

amyangfei commented May 16, 2019

csuzhangxc commented May 20, 2019

IANTHEREAL May 21, 2019

csuzhangxc May 21, 2019

IANTHEREAL May 21, 2019

IANTHEREAL commented May 21, 2019

IANTHEREAL left a comment

*: add Writer for relay log #117

*: add Writer for relay log #117

Conversation

csuzhangxc commented Apr 18, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

csuzhangxc commented Apr 22, 2019

csuzhangxc commented May 13, 2019

csuzhangxc commented May 13, 2019

codecov bot commented May 13, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IANTHEREAL commented May 16, 2019

amyangfei commented May 16, 2019

csuzhangxc commented May 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IANTHEREAL commented May 21, 2019

IANTHEREAL left a comment

Choose a reason for hiding this comment

*: add `Writer` for relay log #117

*: add `Writer` for relay log #117

csuzhangxc commented Apr 18, 2019 •

edited

Loading

codecov bot commented May 13, 2019 •

edited

Loading