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

*: implement the TxnHeartBeat API for the large transaction #11979

Merged
merged 19 commits into from
Sep 9, 2019

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

In the implementation of the large transaction, a large transaction may last for a long time, during which we must ensure that the lock of the transaction will not been cleaned up.

So we need a new API in the kvproto to update the TTL of a transaction, namely TxnHeartBeat.

What is changed and how it works?

Implement the TxnHeartBeat API in the mocktikv, add the unit test.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

@codecov
Copy link

codecov bot commented Sep 2, 2019

Codecov Report

Merging #11979 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #11979   +/-   ##
===========================================
  Coverage   81.9618%   81.9618%           
===========================================
  Files           447        447           
  Lines         97848      97848           
===========================================
  Hits          80198      80198           
  Misses        12111      12111           
  Partials       5539       5539

@coocood
Copy link
Member

coocood commented Sep 3, 2019

LGTM

@tiancaiamao tiancaiamao added needs-cherry-pick-3.0 and removed sig/transaction SIG:Transaction labels Sep 5, 2019
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/rebuild

jackysp
jackysp previously approved these changes Sep 5, 2019
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tiancaiamao
Copy link
Contributor Author

/rebuild

@MyonKeminta
Copy link
Contributor

MyonKeminta commented Sep 5, 2019

Is it expected that go.mod and go.sum has so many changes? I think only updating kvproto shouldn't cause so many changes.

@tiancaiamao
Copy link
Contributor Author

Is it expected that go.mod and go.sum has so many changes?

It's expected. @MyonKeminta
go.mod is updated to use the new kvproto package, and go.sum is update automatically.

@tiancaiamao
Copy link
Contributor Author

/run-all-tests tidb-test=pr/881

Copy link
Contributor

@MyonKeminta MyonKeminta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

store/mockstore/mocktikv/mock_tikv_test.go Outdated Show resolved Hide resolved
store/tikv/lock_test.go Show resolved Hide resolved
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-integration-common-test tikv=pr/5407

1 similar comment
@tiancaiamao
Copy link
Contributor Author

/run-integration-common-test tikv=pr/5407

@MyonKeminta
Copy link
Contributor

/run-all-tests tikv=pr/5407

@tiancaiamao tiancaiamao added status/all tests passed status/LGT2 Indicates that a PR has LGTM 2. labels Sep 5, 2019
@MyonKeminta
Copy link
Contributor

/run-all-tests tikv=pr/5407

@MyonKeminta
Copy link
Contributor

/run-unit-test tikv=pr/5407

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@coocood coocood merged commit 53a7cf6 into pingcap:master Sep 9, 2019
@tiancaiamao tiancaiamao deleted the txn-heart-beat branch September 9, 2019 02:09
@sre-bot
Copy link
Contributor

sre-bot commented Sep 9, 2019

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants