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(ticdc): Add output-old-value config #10915

Merged
merged 11 commits into from
Apr 24, 2024

Conversation

sdojjy
Copy link
Member

@sdojjy sdojjy commented Apr 17, 2024

What problem does this PR solve?

Issue Number: close #10916

What is changed and how it works?

this Add sink.open.output-old-value for open protocol, sink.debezium.output-old-value for debezium protocol, the default value is true.

Open Protocol Events behavior detail

If sink.open.output-old-value is true, no difference
If sink.open.output-old-value is false

  • Add
    No impact
{
"u": [{"c1": xxx}],
"p": null,
"d": null
}
  • Update
    The p will be null
{
"u": [{"c1": xxx}],
"p": null,
"d": null
}
  • Delete
    No impact
{
"d": [{"c1": xxx}],
"p": null,
"u": null
}

Debezium protocol behavior

If sink.debezium.output-old-value is true, no difference
If sink.debezium.output-old-value is false

  • Add
    No impact
  • Update
    The before fields is null
  {
    "before": null,
    "after": {
        "id": 111,
        "name": "scooter",
        "description": "Big 2-wheel scooter",
        "weight": 5.15
        },
    "source": {...},
    "op": "u",
    "ts_ms": 1589362330904 ,
    "transaction": null
    };
  • Delete
    No impact

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Add `sink.open.output-old-value`  for open protocol,  `sink.debezium.output-old-value` for debezium protocol to control weather output the old value to downstream.

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. affect-ticdc-config-docs Pull requests that affect TiCDC configuration docs. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Apr 17, 2024
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Merging #10915 (fce6e8a) into master (5ebadff) will increase coverage by 0.0587%.
Report is 3 commits behind head on master.
The diff coverage is 100.0000%.

Additional details and impacted files
Components Coverage Δ
cdc 61.7521% <100.0000%> (+0.1025%) ⬆️
dm 51.2454% <ø> (+0.0141%) ⬆️
engine 63.4232% <ø> (+0.0070%) ⬆️
Flag Coverage Δ
unit 57.4366% <100.0000%> (+0.0587%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master     #10915        +/-   ##
================================================
+ Coverage   57.3779%   57.4366%   +0.0587%     
================================================
  Files           851        851                
  Lines        125137     125225        +88     
================================================
+ Hits          71801      71925       +124     
+ Misses        47944      47901        -43     
- Partials       5392       5399         +7     

@sdojjy sdojjy changed the title sink(ticdc): Add output-old-value config (WIP) sink(ticdc): Add output-old-value config Apr 18, 2024
@ti-chi-bot ti-chi-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 18, 2024
@sdojjy
Copy link
Member Author

sdojjy commented Apr 18, 2024

/retest-required

@sdojjy sdojjy changed the title (WIP) sink(ticdc): Add output-old-value config sink(ticdc): Add output-old-value config Apr 23, 2024
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 23, 2024
@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2024
…lue-config

# Conflicts:
#	pkg/sink/codec/open/open_protocol_encoder_test.go
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2024
@3AceShowHand
Copy link
Contributor

/retest

@sdojjy
Copy link
Member Author

sdojjy commented Apr 23, 2024

/retest-required

@3AceShowHand
Copy link
Contributor

/retest

cdc/api/v2/model.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 23, 2024
Copy link
Contributor

ti-chi-bot bot commented Apr 23, 2024

@flowbehappy: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Collaborator

@hongyunyan hongyunyan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

ti-chi-bot bot commented Apr 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3AceShowHand, flowbehappy, hongyunyan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added lgtm approved and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 23, 2024
Copy link
Contributor

ti-chi-bot bot commented Apr 23, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-04-23 07:46:30.971194271 +0000 UTC m=+73547.711097286: ☑️ agreed by 3AceShowHand.
  • 2024-04-23 09:51:16.082760084 +0000 UTC m=+81032.822662995: ☑️ agreed by hongyunyan.

@hongyunyan
Copy link
Collaborator

/retest

@sdojjy
Copy link
Member Author

sdojjy commented Apr 23, 2024

/retest-required

2 similar comments
@sdojjy
Copy link
Member Author

sdojjy commented Apr 24, 2024

/retest-required

@sdojjy
Copy link
Member Author

sdojjy commented Apr 24, 2024

/retest-required

@sdojjy
Copy link
Member Author

sdojjy commented Apr 24, 2024

/test cdc-integration-kafka-test

@ti-chi-bot ti-chi-bot bot merged commit 295a39a into pingcap:master Apr 24, 2024
29 checks passed
@sdojjy sdojjy deleted the add-output-old-value-config branch April 24, 2024 05:35
@sdojjy sdojjy added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Apr 24, 2024
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-8.1: #10964.

ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Apr 24, 2024
@sdojjy sdojjy added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Apr 25, 2024
ti-chi-bot pushed a commit to ti-chi-bot/tiflow that referenced this pull request Apr 25, 2024
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.5: #10974.

@ti-chi-bot ti-chi-bot removed the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Apr 26, 2024
ti-chi-bot bot pushed a commit that referenced this pull request Apr 28, 2024
ti-chi-bot bot pushed a commit that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affect-ticdc-config-docs Pull requests that affect TiCDC configuration docs. approved lgtm needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sink(ticdc): Add output-old-value config
5 participants