tikv / sig-transaction

Resources for the transaction SIG
63 stars 13 forks source link

Audit usage of commit_ts #19

Closed nrc closed 3 years ago

nrc commented 4 years ago

Look for any places where non-unique commit_ts might cause a problem (for example, it means that we cannot order two transactions with the same commit_ts). In the big picture it means commit_ts only gives a partial order of transactions.

When looking at tools such as CDC and binlog, please also check if there might be other issues with the parallel commit design.

sticnarf commented 4 years ago

TiDB

I found no assumption that commit ts must be unique.


Binlog

It's impossible for binlog to support non-unique commit ts. It cannot handle the case when a transaction's start ts is identical to another transaction's commit ts, either.

We should check that parallel commit and binlog are not enabled at the same time.


CDC

CDC itself does not require every timestamp to be unique. If we have a single write cf record which represents a rollback and a commit, TiKV can still output two events for it. It shouldn't be too complex. /cc @MyonKeminta


Backup & Restore

If the maximum commit ts in the existing backup is T. An incremental backup dumps data with commit ts in (T, +inf).

It is possible that a transaction commits with T after the previous backup. But the next incremental backup skips it.

Note: If we are using max_read_ts + 1 to commit instead of max_ts + 1, it's even possible that the commit ts is small than T, which is more troublesome.


TiKV

See https://github.com/tikv/sig-transaction/pull/16.

TiKV does not compare commit ts across different transactions so I think only the merged write/rollback record needs to be handled carefully.

youjiali1995 commented 4 years ago

For the CDC part, there is a document: https://docs.google.com/document/d/1pbtEx49F9VXJzphCt-PzfypOk_DVKCkD0F3BXoioDO4/edit.

youjiali1995 commented 4 years ago

If the maximum commit ts in the existing backup is T. An incremental backup dumps data with commit ts in (T, +inf).

Does BR check locks? What if a transaction gets a commit_ts and takes a long time to commit?

sticnarf commented 4 years ago

If the maximum commit ts in the existing backup is T. An incremental backup dumps data with commit ts in (T, +inf).

Does BR check locks? What if a transaction gets a commit_ts and takes a long time to commit?

Yes. It resolves all prewritten locks before dumping any data.

But with calculated commit ts, it's possible that we have no pending lock at backup. But max_read_ts may be smaller than the max commit ts.

Possible solution:

nrc commented 3 years ago

Closing since this is all addressed now, other than testing.