tikv / sig-transaction

Resources for the transaction SIG
62 stars 13 forks source link

Compatible issues between async commit and green GC #82

Open youjiali1995 opened 3 years ago

youjiali1995 commented 3 years ago
  1. Green GC is not compatible with async commit if async apply is enabled: A lock isn't applied and the transaction may be committed.
  2. TBD
sticnarf commented 3 years ago

It's a blocker for enabling async apply by default. Maybe not a blocker for async commit GA.

MyonKeminta commented 3 years ago

For ScanLock and PhysicalScanLock requests, it should update the max_ts, check the memory lock, and finally read the raftkv or the rocksdb. For checking memory locks, there are two possible implementations: wait for the memory lock being released, or return them to the client. But for the second way, the implementation should carefully keep the increasing order of the keys in the response, and do not return the found memory locks that are greater than max lock from rocksdb (to avoid missing range since tidb will continue scanning from the last key in the response).

MyonKeminta commented 3 years ago

As for async apply prewrite, can we let TiKV simply return error for green gc requests if async apply prewrite is enabled?

youjiali1995 commented 3 years ago

There is another corner case that green GC doesn't solve now.. https://github.com/tikv/tikv/issues/8184#issuecomment-763483003

MyonKeminta commented 3 years ago

Ah.. yes.. and

As for async apply prewrite, can we let TiKV simply return error for green gc requests if async apply prewrite is enabled?

I notice that this is not a good idea considering the case that TiKV's config is changed and restarted.

Recently Jay discussed with me about a new idea that maybe we can use resolved_ts to determine if there's any lock before safepoint that needs to be resolved. Since resolved_ts will also be used by follower stale read, it may be an always-running service, instead of running only if needed by CDC. I think this is a possible substitution to green gc, but sounds hard to deliver it in 5.0 GA.