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

Is delayForAsyncCommit needed before verify records by foreign_key and constraints check? #48297

Closed
jiyfhust opened this issue Nov 4, 2023 · 3 comments · Fixed by #56349
Closed
Labels
severity/moderate sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.

Comments

@jiyfhust
Copy link
Contributor

jiyfhust commented Nov 4, 2023

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

I see there is delayForAsyncCommit before verifying records, for example:

tidb/pkg/ddl/partition.go

Lines 2496 to 2498 in 4f00ece

if d.lease > 0 {
delayForAsyncCommit()
}

But there is no delay used by foreign_key and constraints check.

func checkForeignKeyConstrain(w *worker, schema, table string, fkInfo *model.FKInfo, fkCheck bool) error {

func (w *worker) verifyRemainRecordsForCheckConstraint(dbInfo *model.DBInfo, tableInfo *model.TableInfo, constr *model.ConstraintInfo, job *model.Job) error {

Is it a potential bug under async commit?

I don't known how to reproduce it to verify this question.

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiDB version? (Required)

master

@jiyfhust jiyfhust added the type/bug The issue is confirmed as a bug. label Nov 4, 2023
@jiyfhust
Copy link
Contributor Author

jiyfhust commented Nov 4, 2023

PTAL @crazycs520

@YangKeao
Copy link
Member

YangKeao commented Sep 26, 2024

I've reproduced this issue by adding a new failpoint to the onCreateForeignKey function:

	case model.StateWriteOnly:
		err = checkForeignKeyConstrain(w, job.SchemaName, tblInfo.Name.L, &fkInfo, fkCheck)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, err
		}
		failpoint.Eval(_curpkg_("afterCheckForeignKeyConstrain"))
		tblInfo.ForeignKeys[len(tblInfo.ForeignKeys)-1].State = model.StateWriteReorganization
		ver, err = updateVersionAndTableInfo(jobCtx, t, job, tblInfo, true)
		if err != nil {
			return ver, errors.Trace(err)
		}
		job.SchemaState = model.StateWriteReorganization

Prepare:

  1. Set system variables:
    1. set global tidb_enable_async_commit = 'ON'
    2. set global tidb_enable_1pc='OFF'
    3. set global tidb_enable_metadata_lock='OFF'
  2. Set the failpoint:
    1. curl -X PUT http://127.0.0.1:10080/fail/tikvclient/beforePrewrite --data "sleep(1500)"
    2. curl -X PUT http://127.0.0.1:10080/fail/github.com/pingcap/tidb/pkg/ddl/afterCheckForeignKeyConstrain --data "sleep(3000)"
    3. curl -X PUT http://127.0.0.1:10080/fail/tikvclient/asyncCommitDoNothing --data "sleep(50000)".
  3. Create the table in test database
    1. create table child (id int primary key, val int, index(val));
    2. create table parent (id int primary key, val int, index(val));

Run:
Start two mysql client and connect to the same TiDB node.

Execute this SQL in the first connections: insert into child values (1, 1)
Before this SQL statement finishes (you'll have 1 second), execute the DDL in the other connection alter table child add foreign key fk(val) references parent (val);.

Then both of these SQLs will succeed, and the foreign key constraint is broken.

@jiyfhust
Copy link
Contributor Author

I've reproduced this issue by adding a new failpoint to the onCreateForeignKey function:

	case model.StateWriteOnly:
		err = checkForeignKeyConstrain(w, job.SchemaName, tblInfo.Name.L, &fkInfo, fkCheck)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, err
		}
		failpoint.Eval(_curpkg_("afterCheckForeignKeyConstrain"))
		tblInfo.ForeignKeys[len(tblInfo.ForeignKeys)-1].State = model.StateWriteReorganization
		ver, err = updateVersionAndTableInfo(jobCtx, t, job, tblInfo, true)
		if err != nil {
			return ver, errors.Trace(err)
		}
		job.SchemaState = model.StateWriteReorganization

Prepare:

  1. Set system variables:

    1. set global tidb_enable_async_commit = 'ON'
    2. set global tidb_enable_1pc='OFF'
    3. set global tidb_enable_metadata_lock='OFF'
  2. Set the failpoint:

    1. curl -X PUT http://127.0.0.1:10080/fail/tikvclient/beforePrewrite --data "sleep(1500)"
    2. curl -X PUT http://127.0.0.1:10080/fail/github.com/pingcap/tidb/pkg/ddl/afterCheckForeignKeyConstrain --data "sleep(3000)"
    3. curl -X PUT http://127.0.0.1:10080/fail/tikvclient/asyncCommitDoNothing --data "sleep(50000)".
  3. Create the table in test database

    1. create table child (id int primary key, val int, index(val));
    2. create table parent (id int primary key, val int, index(val));

Run: Start two mysql client and connect to the same TiDB node.

Execute this SQL in the first connections: insert into child values (1, 1) Before this SQL statement finishes (you'll have 1 second), execute the DDL in the other connection alter table child add foreign key fk(val) references parent (val);.

Then both of these SQLs will succeed, and the foreign key constraint is broken.

Great Great!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/moderate sig/transaction SIG:Transaction type/bug The issue is confirmed as a bug.
Projects
None yet
4 participants