*: implement the CheckTxnStatus API for the large transaction
#11974
Conversation
tiancaiamao
added
sig/transaction
sticnarf
requested review from
youjiali1995
and removed request for
sticnarf
Codecov Report
@@ Coverage Diff @@
## master #11974 +/- ##
===========================================
Coverage 81.4652% 81.4652%
===========================================
Files 451 451
Lines 97379 97379
===========================================
Hits 79330 79330
Misses 12400 12400
Partials 5649 5649 |
|
LGTM |
|
|
||
| // If this is a large transaction and the lock is active, push forward the minCommitTS. | ||
| // lock.minCommitTS == 0 may be a secondary lock, or not a large transaction. | ||
| if lock.minCommitTS > 0 { |
There was a problem hiding this comment.
does ther caller need to know if it did update the minCommitTS ?
There was a problem hiding this comment.
I think it's unnecessary @cfzjywxk
| value: []byte{'d', 'e'}, | ||
| op: kvrpcpb.Op_Put, | ||
| ttl: 444, | ||
| minCommitTS: 666, |
There was a problem hiding this comment.
Need check in the below
c.Assert(l.minCommitTS, Equals, l1.minCommitTS)
| ttl, commitTS, err = s.store.CheckTxnStatus([]byte("pk1"), 5, 0, 666) | ||
| c.Assert(err, IsNil) | ||
| c.Assert(ttl, Equals, uint64(0)) | ||
| c.Assert(commitTS, Equals, uint64(0)) |
There was a problem hiding this comment.
Add test for minCommitTS logic?
There was a problem hiding this comment.
The minCommitTS is not returned in the CheckTxnStatus response, so we can't check it here.
Anyway, in the following PR we'll cover the minCommitTS
@crazycs520
| @@ -370,7 +373,7 @@ func (lr *LockResolver) getTxnStatus(bo *Backoffer, txnID uint64, primary []byte | |||
| return status, err | |||
| } | |||
| if cmdResp.CommitVersion != 0 { | |||
| status = TxnStatus(cmdResp.GetCommitVersion()) | |||
| status = TxnStatus{0, cmdResp.GetCommitVersion()} | |||
There was a problem hiding this comment.
ttl is always 0? change this later or?
There was a problem hiding this comment.
In the CleanUp proto, the result would be either committed or rollbacked, so the ttl is always 0.
|
PTAL @crazycs520 @cfzjywxk |
|
LGTM |
tiancaiamao
added
status/LGT2
|
/run-all-tests |
What problem does this PR solve?
Implement a new kvproto API for the incoming large transaction support.
What is changed and how it works?
In the new kvproto, there will be a
minCommitTSfield, the large transaction will use it.CheckTxnStatuschecks the status of a transaction(on the primary key).If the lock exists and is valid,
CheckTxnStatuswould push forward theminCommitTSusing the current reader'sstartTS.In this commit, I implement the API in the mocktikv, adding unit test, covering the storage and proto parts.
Check List
Tests
Code changes