Dangling delete record when deletes your own write in a transaction

[youjiali1995]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

See https://github.com/pingcap/ticdc/issues/2612. This behavior of TiDB may result in strange event in TiCDC.

mysql root@127.0.0.1:test> create table t2 (id varchar(32) not null primary key, a int);
Query OK, 0 rows affected

mysql root@127.0.0.1:test> begin;
Query OK, 0 rows affected

mysql root@127.0.0.1:test> insert into t2 values (2,2);
Query OK, 1 row affected

mysql root@127.0.0.1:test> delete from t2 where id = 2;
Query OK, 1 row affected

mysql root@127.0.0.1:test> commit;
Query OK, 0 rows affected

[10:59:42] youjiali1995:~ $ curl http://127.0.0.1:10080/mvcc/key/test/t2/1
{
 "key": "74800000000000004E5F728000000000000001",
 "region_id": 2,
 "value": {
  "info": {
   "writes": [
    {
     "type": 1,
     "start_ts": 427258114963668993,
     "commit_ts": 427258119341473794
    }
   ]
  }
 }
} 

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

No delete record in write CF.

3. What did you see instead (Required)

Only When deletes by the clustered primary key or unique index key in optimistic transactions, the delete record on such keys won't appear.
https://github.com/tikv/client-go/blob/16d902a3c7e5e850c931f0e9515c3dbb4944b6f8/txnkv/transaction/2pc.go#L435-L440

4. What is your TiDB version? (Required)

4.0.x, 5.x

[cfzjywxk]

@TonsnakeLin
PTAL, this unncessary record may affect the ticdc data processing.

[cfzjywxk]

Currently in tidb the "delete-your-write" tranaction commit is not fully optimized, just the rowkey or unique index keys in the optimistic transaction satisfying the "delete-your-write" condition will be ignored, which also introduces some critical bugs before.

If we want to solve it completely we may need to find out a way to distinguish the "newly" inserted and later deleted keys for both optimistic and pessimistic transactions, also this may be a risky change so we need to plan it well before doing things.

[cfzjywxk]

There are serveral things to consider deciding if the key in the memory buffer is "newly" inserted and then deleted. The difficult part is to diffrentiate the following situation:

// prepare
create table t(c1 int primary key, c2 int, key k1(c2));
insert into t values(1, 1);

// test
begin;
delete from t;
insert into t values(1, 2);
delete from t;
commit

Here the final generated delete operation could not be ignored though it deletes the new row inserted by previous insert statement.

For the pessimistic transaction mode, a simple way to verify if the inserted key is "new" is to verify if the oldest value for this key in memdb is a valid put operation. As key existency check is done executing each DML statement for pessimistic transactions, a valid PUT means the key is inserted by the transaction itself.

For the optimistic transactions with default configurations, the row key or the unique index key would be marked with "presume not exist flag" and they will be skipped in the 2pc phase processing the delete mutations.However the related secondary index key delete opertion is not skipped and they will still be generated and forwarded as part of the transaction mutations.A simple way to solve this is add similar mark on the secondary index keys too so they could be skippped in the delete-your-write situation too.

For the optimistic transaction with tidb_constraint_check_in_place enabled, the same solution could be used like the pessimistic transactions.

/cc @MyonKeminta @you06 @coocood
PTAL

[you06]

A simple way to solve this is add similar mark on the secondary index keys too so they could be skippped in the delete-your-write situation too.

If so, it seems that the case you given will commit success in the optimistic transaction, is this expected?

[MyonKeminta]

I have one more question: when we avoid the delete record on a delete-your-write key, should we still put a Lock record here to avoid breaking the isolation level?

[cfzjywxk]

I have one more question: when we avoid the delete record on a delete-your-write key, should we still put a Lock record here to avoid breaking the isolation level?

@MyonKeminta @you06

I think so, though the current implementation is a bit confusing processing delete-your-write.

For optimistic transactions with tidb_constraint_check_in_place off(default config):

• Rowkey: the Op_CheckNotExist opertion type will be used, so nothing will be written.
• Secondary index key: Op_Del will be used and there wiil be a delete record for it.

For optimistic transactions with tidb_constraint_check_in_place on:

• All the keys will be written using Op_Del type.

For pessimistic transactions, the pessimistic lock will be added first doing insert, and then delete will change the record in memdb into a delete type, doing commit:

• All the keys will be written using Op_Del type which is what the issue describes.

The resonable way is that the locked keys should be committed using lock type and other keys deleted should be skipped. The difficult part is how to identify the delete-your-write "deletes" mentioned in the above comment.