Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

decode: support new format of row data #241

Merged
merged 8 commits into from
Feb 6, 2020
Merged

decode: support new format of row data #241

merged 8 commits into from
Feb 6, 2020

Conversation

zier-one
Copy link
Contributor

What problem does this PR solve?

Ref: pingcap/tidb#12634
https://github.com/pingcap/tidb/blob/master/docs/design/2018-07-19-row-format.md

support new format of row data

What is changed and how it works?

  1. update tidb version
  2. check the version of row data format
  3. decode row data in new format
  4. make the behaviour of these two functions consistent:
    decodeRowV1, decodeRowV2

Check List

Tests

  • Unit test
  • Integration test

@zier-one zier-one added the WIP label Jan 16, 2020
@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one zier-one added status/ptal Could you please take a look? and removed WIP labels Jan 17, 2020
@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one
Copy link
Contributor Author

/run-all-tests

2 similar comments
@zier-one
Copy link
Contributor Author

/run-all-tests

@zier-one
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

codecov-io commented Jan 19, 2020 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #241   +/-   ##
========================================
  Coverage          ?   48.9%           
========================================
  Files             ?      50           
  Lines             ?    4466           
  Branches          ?       0           
========================================
  Hits              ?    2184           
  Misses            ?    2046           
  Partials          ?     236
Impacted Files Coverage Δ
cdc/capture.go 0% <0%> (ø)
cdc/processor.go 13.44% <0%> (ø)
cdc/kv/client.go 9.21% <0%> (ø)
pkg/util/region.go 100% <100%> (ø)
cdc/kv/matcher.go 100% <100%> (ø)
cdc/capture_info.go 75% <100%> (ø)
cdc/roles/manager.go 70.3% <25%> (ø)
cdc/owner.go 38.12% <50%> (ø)
cdc/scheduler.go 57.57% <75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34ece8b...33f62dc. Read the comment docs.

@zier-one zier-one requested a review from suzaku February 3, 2020 02:56
suzaku
suzaku reviewed Feb 3, 2020
View reviewed changes
cdc/capture.go Outdated Show resolved Hide resolved
cdc/capture.go Outdated Show resolved Hide resolved
cdc/kv/client.go Outdated Show resolved Hide resolved
leoppro added 3 commits February 4, 2020 16:57
update
34ece8b
Merge remote-tracking branch 'pingcap/master' into new_row
39d6e94 
# Conflicts:
#	cdc/capture.go
#	cdc/capture_info.go
#	cdc/capture_info_test.go
#	cdc/owner.go
#	cdc/scheduler.go
#	cdc/scheduler_test.go
#	go.mod
#	go.sum
update
33f62dc
@zier-one
Copy link
Contributor Author

zier-one commented Feb 4, 2020

@suzaku PTAL again

@zier-one
Copy link
Contributor Author

zier-one commented Feb 4, 2020

/run-all-tests

lysu
lysu reviewed Feb 4, 2020
View reviewed changes
go.mod Outdated Show resolved Hide resolved
suzaku
suzaku reviewed Feb 5, 2020
View reviewed changes
Copy link
Contributor

@suzaku suzaku left a comment

Choose a reason for hiding this comment

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

LGTM

@suzaku suzaku added LGT1 and removed status/ptal Could you please take a look? labels Feb 5, 2020
amyangfei
amyangfei reviewed Feb 5, 2020
View reviewed changes
cdc/puller/mock_puller_test.go Outdated Show resolved Hide resolved
cdc/schema/storage.go Show resolved Hide resolved
cdc/entry/codec.go Show resolved Hide resolved
@zier-one
Copy link
Contributor Author

zier-one commented Feb 6, 2020

/run-all-tests

amyangfei
amyangfei approved these changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@amyangfei amyangfei left a comment

Choose a reason for hiding this comment

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

LGTM

@amyangfei
Copy link
Contributor

btw, shall we upgrade the TiDB binary in jenkins integration test to the latest?

@amyangfei amyangfei added LGT2 and removed LGT1 labels Feb 6, 2020
@zier-one
Copy link
Contributor Author

zier-one commented Feb 6, 2020

btw, shall we upgrade the TiDB binary in jenkins integration test to the latest?

I will do that and add an integration test for row format compatibility

@zier-one zier-one merged commit 2837c33 into pingcap:master Feb 6, 2020
@zier-one zier-one deleted the new_row branch February 6, 2020 06:40
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
decode: support new format of row data (pingcap#241)
13318d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lysu lysu lysu left review comments

@suzaku suzaku suzaku left review comments

@amyangfei amyangfei amyangfei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zier-one @codecov-io @amyangfei @suzaku @lysu