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.

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

sink: support to output old value #708

Merged
merged 28 commits into from
Aug 3, 2020
Merged

sink: support to output old value #708

merged 28 commits into from
Aug 3, 2020

Conversation

5kbpers
Copy link
Contributor

@5kbpers 5kbpers commented Jul 1, 2020

Signed-off-by: 5kbpers [email protected]

What problem does this PR solve?

Address a part of https://github.com/pingcap/ticdc/issues/690
Blocked by tikv/tikv#8201

Support to output old value

What is changed and how it works?

  • New option for outputting the old value

Check List

Tests (TODO)

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has interface methods change

Side effects

  • Possible performance regression
  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release note

  • sink: support to output old value

@5kbpers 5kbpers changed the title sink: support to output with tidb-binlog protocol sink: support to output old value Jul 1, 2020
@zier-one zier-one added the status/ptal Could you please take a look? label Jul 24, 2020
@zier-one
Copy link
Contributor

please resolve the conflicts @5kbpers

cdc/entry/mounter.go Outdated Show resolved Hide resolved
cdc/entry/mounter.go Show resolved Hide resolved
cdc/entry/mounter.go Outdated Show resolved Hide resolved
cdc/model/sink.go Outdated Show resolved Hide resolved
cdc/puller/puller.go Outdated Show resolved Hide resolved
pkg/regionspan/span.go Show resolved Hide resolved
cdc/processor.go Outdated Show resolved Hide resolved
cdc/entry/mounter.go Outdated Show resolved Hide resolved
cdc/entry/mounter.go Outdated Show resolved Hide resolved
cdc/model/sink.go Outdated Show resolved Hide resolved
cdc/processor.go Outdated Show resolved Hide resolved
cdc/puller/puller.go Outdated Show resolved Hide resolved
pkg/regionspan/span.go Outdated Show resolved Hide resolved
cdc/owner_operator.go Outdated Show resolved Hide resolved
cdc/processor.go Outdated Show resolved Hide resolved
@@ -17,6 +17,7 @@ package config
type SinkConfig struct {
DispatchRules []*DispatchRule `toml:"dispatchers" json:"dispatchers"`
Protocol string `toml:"protocol" json:"protocol"`
OldValue bool `toml:"old-value" json:"old-value"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we add old-value config to cmd/changefeed.toml now, and add comment to address this is an experimental feature.

@zier-one
Copy link
Contributor

/run-integration-tests

@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-integration-tests

@amyangfei amyangfei added the release-blocker This issue blocks a release. Please solve it ASAP. label Aug 3, 2020
@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-integration-tests

leoppro added 2 commits August 3, 2020 11:26
# Conflicts:
#	cdc/sink/mysql.go
#	go.mod
#	go.sum
@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-integration-tests

@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-integration-tests

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2020

Codecov Report

Merging #708 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master       #708   +/-   ##
===========================================
  Coverage   33.2346%   33.2346%           
===========================================
  Files            96         96           
  Lines         11479      11479           
===========================================
  Hits           3815       3815           
  Misses         7309       7309           
  Partials        355        355           

@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-kafka-tests

@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-kafka-tests

# Conflicts:
#	cdc/entry/mounter.go
@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/run-all-tests

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

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 3, 2020
@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/lgtm

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 3, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 3, 2020
@zier-one
Copy link
Contributor

zier-one commented Aug 3, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 3, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 62e246f into pingcap:master Aug 3, 2020
@zier-one zier-one mentioned this pull request Oct 28, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker This issue blocks a release. Please solve it ASAP. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. status/ptal Could you please take a look?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants