cdc: remove assertion on the lock start-ts by 3AceShowHand · Pull Request #19084 · tikv/tikv

3AceShowHand · 2025-11-04T07:11:24Z

What is changed and how it works?

Issue Number: Close #19083

What's Changed:

* revert #16864 to avoid the TiKV-CDC panic due to assertion failed

The generation field is removed by revert the PR, need to do more work about how to support pipelined DML.
When push_lock, just push it like before, and print logs if there is already the lock with the same key.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

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

Release note

revert #16864 to avoid the TiKV-CDC panic due to assertion failed

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

This reverts commit d99bd9b. Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

components/cdc/src/delegate.rs

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand · 2025-11-06T09:08:38Z

/test pull-unit-test

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand · 2025-11-06T09:52:50Z

/test pull-unit-test

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

cfzjywxk · 2025-11-07T02:21:01Z

@3AceShowHand
It seems the DELETE LOCK event on non-protected-rollback lock key is not handled in the current PR yet?

cfzjywxk · 2025-11-07T02:21:55Z

/retest

3AceShowHand · 2025-11-07T03:48:30Z

/test pull-unit-test

3AceShowHand · 2025-11-07T03:50:21Z

@3AceShowHand It seems the DELETE LOCK event on non-protected-rollback lock key is not handled in the current PR yet?

Yes, I am investigating how to handle the DELETE LOCK and also ignore the pessimistic locks, to avoid the #17876

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand · 2025-11-07T05:15:06Z

components/cdc/src/delegate.rs

-                    assert_eq!(row.lock_count_modify, 0);
                    let start_ts = TimeStamp::from(row.v.start_ts);
-                    row.lock_count_modify = self.pop_lock(key, start_ts)?;
+                    let mut modified = self.pop_lock(key, start_ts)?;


This is a important change.

Previous code assert_eq!(row.lock_count_modify, 0); make one assumption that there is no 2 put request with the same key in the one batch.

The assertion is removed, and make that it is possible the case above happens. For example, lock cf put key a, and then write cf pop key a.

3AceShowHand · 2025-11-07T06:16:21Z

/retest

cfzjywxk

LGTM, we may leave the lock cf handling compalitity refactor in the later PR

ti-chi-bot · 2025-11-07T07:17:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, overvenus

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cfzjywxk,overvenus]

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 · 2025-11-07T07:17:17Z

[LGTM Timeline notifier]

Timeline:

2025-11-07 06:53:06.901766863 +0000 UTC m=+425836.344796741: ☑️ agreed by cfzjywxk.
2025-11-07 07:17:15.850791462 +0000 UTC m=+427285.293821341: ☑️ agreed by overvenus.

3AceShowHand · 2025-11-07T07:39:37Z

/cherry-pick release-8.5

close tikv#19083 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2025-11-07T07:40:15Z

@3AceShowHand: new pull request created to branch release-8.5: #19096.
But this PR has conflicts, please resolve them!

Details

In response to this:

/cherry-pick release-8.5

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 ti-community-infra/tichi repository.

close #19083 * revert #16864 to avoid the TiKV-CDC panic due to assertion failed Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: 3AceShowHand <jinl1037@hotmail.com> Co-authored-by: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Co-authored-by: 3AceShowHand <jinl1037@hotmail.com>

close tikv#19083 * revert tikv#16864 to avoid the TiKV-CDC panic due to assertion failed Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand added 2 commits November 4, 2025 21:08

remove assertion on the lock start-ts

20aced0

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Revert "cdc: handles unordered prewrites on one same key (tikv#16864)"

379ed43

This reverts commit d99bd9b. Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand force-pushed the cdc-remove-assert branch from 2f787b6 to 379ed43 Compare November 4, 2025 13:08

add more logs to detect start-ts not equal scenario

bb8d0a7

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

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 Nov 4, 2025

3AceShowHand added 2 commits November 4, 2025 22:03

fix the panic

4a4f999

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add one unit test

1ae39f9

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 5, 2025

lcwangchao reviewed Nov 5, 2025

View reviewed changes

components/cdc/src/delegate.rs Outdated Show resolved Hide resolved

3AceShowHand added 13 commits November 5, 2025 14:28

do not update resolved-ts

367b59c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add one more logs

d7ff043

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add log to indicate overlapped rollback

c32b849

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add log to indicate overlapped rollback

580d17d

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add more logs

5ff08ac

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

pop lock even though the start-ts not match

2744575

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

adjust some code to avoid ownership

61bdd4c

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

do not overwrite lock with the same key

c916014

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Add more logs to detect push pop locks

1f26850

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add log to detect why the modified count not 0

a634a89

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add batch id to rows in build

2e3fddf

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

pop lock by check start-ts to avoid panic

457fe2b

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

add lock modified count struct

eeb769b

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

ti-chi-bot bot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 5, 2025

remove debug logs, ready to be reviewed

c5012d9

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

remove debug logs, ready to be reviewed

62f25a1

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

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 Nov 6, 2025

add more logs

e512e5e

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

do not assume the key is not the same within the same request batch

37b2b46

Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

3AceShowHand commented Nov 7, 2025

View reviewed changes

cfzjywxk approved these changes Nov 7, 2025

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Nov 7, 2025

overvenus approved these changes Nov 7, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Nov 7, 2025

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 7, 2025

ti-chi-bot bot merged commit 887e6c8 into tikv:master Nov 7, 2025
9 checks passed

ti-chi-bot bot added this to the Pool milestone Nov 7, 2025

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Nov 7, 2025

This is an automated cherry-pick of tikv#19084

740c747

close tikv#19083 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Nov 7, 2025

cdc: remove assertion on the lock start-ts (#19084) #19096

Merged

9 tasks

rxdoi pushed a commit to rxdoi/tikv that referenced this pull request Nov 18, 2025

cdc: remove assertion on the lock start-ts (tikv#19084)

ccae383

close tikv#19083 * revert tikv#16864 to avoid the TiKV-CDC panic due to assertion failed Signed-off-by: 3AceShowHand <jinl1037@hotmail.com>

Conversation

3AceShowHand commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

Uh oh!

3AceShowHand commented Nov 6, 2025

Uh oh!

3AceShowHand commented Nov 6, 2025

Uh oh!

cfzjywxk commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cfzjywxk commented Nov 7, 2025

Uh oh!

3AceShowHand commented Nov 7, 2025

Uh oh!

3AceShowHand commented Nov 7, 2025

Uh oh!

3AceShowHand Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

3AceShowHand commented Nov 7, 2025

Uh oh!

cfzjywxk left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Nov 7, 2025

Uh oh!

ti-chi-bot bot commented Nov 7, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

3AceShowHand commented Nov 7, 2025

Uh oh!

ti-chi-bot commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

3AceShowHand commented Nov 4, 2025 •

edited

Loading

cfzjywxk commented Nov 7, 2025 •

edited

Loading