lightning: support data KV checking of 'replace' mode for lightning post-import conflict detection #46763

lyzx2001 · 2023-09-07T14:23:10Z

What problem does this PR solve?

Issue Number: close #45774

Problem Summary:

Currently lightning only supports "remove" mode for post-import conflict detection, but many customers request lightning to support "replace" mode for lightning post-import conflict detection.

We would like to support "replace" mode for lightning post-import conflict detection:
To resolve rows with conflict, instead of deleting all the rows that are engaged in conflict (the algorithm for remove), we delete some of the rows with conflict and reserve other rows that can be kept and not cause conflict anymore. Under this circumstance, we only delete the necessary rows to resolve conflicts, so that we can keep more original rows than remove mode as long as the conflicts are resolved.

The algorithms for index KV checking is contained in #45926

This PR contains the algorithms for data KV checking.

What is changed and how it works?

Demo code for 'replace' mode of lightning post-import conflict detection:

https://github.com/lyzx2001/tidb-conflict-replace

Check List

Tests

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

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Lightning post-import conflict detection has supported "replace" mode, which is to delete some rows with conflict and reserve other rows that can be kept and not cause conflict anymore.

In comparison, the algorithm for "remove" mode is to delete all the rows that are engaged in conflict.

…ost-import conflict detection

codecov · 2023-09-07T14:49:24Z

Codecov Report

Merging #46763 (cfb7250) into master (cc42e4b) will increase coverage by 0.3830%.
Report is 1 commits behind head on master.
The diff coverage is 50.5376%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #46763        +/-   ##
================================================
+ Coverage   72.2961%   72.6792%   +0.3830%     
================================================
  Files          1352       1373        +21     
  Lines        400865     407573      +6708     
================================================
+ Hits         289810     296221      +6411     
- Misses        91856      92557       +701     
+ Partials      19199      18795       -404

Flag	Coverage Δ
integration	`38.6750% <0.0000%> (?)`
unit	`72.2831% <50.5376%> (-0.0131%)`	⬇️

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

Components	Coverage Δ
dumpling	`53.9913% <ø> (ø)`
parser	`84.7651% <ø> (ø)`
br	`48.6735% <50.5376%> (-4.4108%)`	⬇️

lyzx2001 · 2023-09-08T08:50:40Z

/cc @lance6716

lyzx2001 · 2023-09-09T09:32:08Z

/retest

lyzx2001 · 2023-09-09T09:51:51Z

/retest

lance6716 · 2023-09-11T04:07:09Z

/test pull-br-integration-test

tiprow · 2023-09-11T04:07:33Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

lance6716

I'd like to add more test in this PR or in next PR.
For unit tests, the criteria is to improve the coverage. It's helpful to force us to think which corner cases can cover the code path.
For integration tests, we will learn if related API behaviour is expected, like what's the error of "key not exist" using fnGetLatest

br/pkg/lightning/errormanager/errormanager.go

br/pkg/lightning/errormanager/errormanager_test.go

br/pkg/lightning/errormanager/errormanager.go

lyzx2001 · 2023-09-13T10:48:26Z

/test pull-br-integration-test

tiprow · 2023-09-13T10:48:43Z

@lyzx2001: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

lyzx2001 · 2023-09-13T12:55:32Z

/retest

lance6716 · 2023-09-14T04:48:13Z

/test pull-br-integration-test

tiprow · 2023-09-14T04:48:49Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

lyzx2001 · 2023-09-15T06:44:20Z

/test pull-br-integration-test

tiprow · 2023-09-15T06:44:37Z

@lyzx2001: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test tiprow_fast_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

lyzx2001 · 2023-09-15T07:52:57Z

/retest

lance6716 · 2023-09-28T07:14:25Z

/approve

lance6716 · 2023-09-28T07:14:33Z

/test pull-br-integration-test

ti-chi-bot · 2023-09-28T07:14:36Z

@lance6716: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test canary-scan-security
/test check-dev
/test check-dev2
/test mysql-test
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-br-integration-test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-ddl-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-prisma-test
/test pull-mysql-client-test
/test pull-sqllogic-test
/test pull-tiflash-test
/test unit-test

The following commands are available to trigger optional jobs:

/test pull-notify-when-compatibility-sections-changed

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test
pull-notify-when-compatibility-sections-changed

In response to this:

/test pull-br-integration-test

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.

lance6716 · 2023-09-28T07:15:05Z

/test pull-br-integration-test

tiprow · 2023-09-28T07:15:10Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-br-integration-test

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.

tiprow · 2023-09-28T07:15:34Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-br-integration-test

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.

ti-chi-bot · 2023-09-28T07:46:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, D3Hunter, lance6716

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:

~~br/OWNERS~~ [3pointer]

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

lance6716 · 2023-09-28T07:56:33Z

/retest

tiprow · 2023-09-28T07:57:05Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lance6716 · 2023-09-28T08:14:44Z

/retest

tiprow · 2023-09-28T08:15:14Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lance6716 · 2023-09-28T09:36:02Z

/retest

tiprow · 2023-09-28T09:36:31Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lyzx2001 · 2023-09-29T06:37:02Z

/retest

tiprow · 2023-09-29T06:37:37Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lyzx2001 · 2023-10-07T05:33:46Z

/retest

tiprow · 2023-10-07T05:34:17Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lance6716 · 2023-10-07T06:31:57Z

/retest

tiprow · 2023-10-07T06:32:26Z

@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

merge master

lyzx2001 · 2023-10-07T07:42:51Z

/retest

tiprow · 2023-10-07T07:43:20Z

@lyzx2001: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

lightning: support data KV checking of 'replace' mode for lightning p…

61218e7

…ost-import conflict detection

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 7, 2023

update unit tests

4ac742a

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Sep 8, 2023

update unit tests

e653151

ti-chi-bot bot requested a review from lance6716 September 8, 2023 08:50

update unit tests

f881845

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 9, 2023

lance6716 reviewed Sep 11, 2023

View reviewed changes

lyzx2001 added 2 commits September 13, 2023 17:35

update according to comments

3075acc

add more INDEX

bb1ddd4

lyzx2001 requested a review from lance6716 September 13, 2023 11:30

update algorithms of checking data KV

7d55ced

lance6716 added the approved label Sep 28, 2023

ti-chi-bot bot removed the approved label Sep 28, 2023

lance6716 approved these changes Sep 28, 2023

View reviewed changes

3pointer approved these changes Sep 28, 2023

View reviewed changes

ti-chi-bot bot added the approved label Sep 28, 2023

Merge branch 'master' into issue45774-step3

cfb7250

merge master

ti-chi-bot bot merged commit 32d7626 into pingcap:master Oct 7, 2023

lyzx2001 mentioned this pull request Oct 8, 2023

lightning: add complex integration tests for lightning post-import conflict detection "replace" mode #47460

Merged

13 tasks

lyzx2001 mentioned this pull request Nov 10, 2023

lightning: add complex unit tests for lightning post-import conflict detection "replace" mode #48514

Merged

13 tasks

lightning: support data KV checking of 'replace' mode for lightning post-import conflict detection #46763

lightning: support data KV checking of 'replace' mode for lightning post-import conflict detection #46763

Conversation

lyzx2001 commented Sep 7, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

codecov bot commented Sep 7, 2023 • edited Loading

Codecov Report

lyzx2001 commented Sep 8, 2023

lyzx2001 commented Sep 9, 2023

lyzx2001 commented Sep 9, 2023

lance6716 commented Sep 11, 2023

tiprow bot commented Sep 11, 2023

lance6716 left a comment

Choose a reason for hiding this comment

lyzx2001 commented Sep 13, 2023

tiprow bot commented Sep 13, 2023

lyzx2001 commented Sep 13, 2023

lance6716 commented Sep 14, 2023

tiprow bot commented Sep 14, 2023

lyzx2001 commented Sep 15, 2023

tiprow bot commented Sep 15, 2023

lyzx2001 commented Sep 15, 2023

lance6716 commented Sep 28, 2023

lance6716 commented Sep 28, 2023

ti-chi-bot bot commented Sep 28, 2023

lance6716 commented Sep 28, 2023

tiprow bot commented Sep 28, 2023

tiprow bot commented Sep 28, 2023

ti-chi-bot bot commented Sep 28, 2023

lance6716 commented Sep 28, 2023

tiprow bot commented Sep 28, 2023

lance6716 commented Sep 28, 2023

tiprow bot commented Sep 28, 2023

lance6716 commented Sep 28, 2023

tiprow bot commented Sep 28, 2023

lyzx2001 commented Sep 29, 2023

tiprow bot commented Sep 29, 2023

lyzx2001 commented Oct 7, 2023

tiprow bot commented Oct 7, 2023

lance6716 commented Oct 7, 2023

tiprow bot commented Oct 7, 2023

lyzx2001 commented Oct 7, 2023

tiprow bot commented Oct 7, 2023

lyzx2001 commented Sep 7, 2023 •

edited

Loading

codecov bot commented Sep 7, 2023 •

edited

Loading