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

store/tikv: Make maxTxnTimeUse a configurable value (#8215) #8209

Merged

Conversation

MyonKeminta
Copy link
Contributor

Signed-off-by: MyonKeminta [email protected]

What problem does this PR solve?

Sometimes in an olap senario, an insert into select statement may take too long time to finish. Then the maxTxnTimeUse (590 seconds) may easily be exceeded. It's better to make it configurable.

The maxTxnTimeUse is required because we don't want the transaction be broken by GC. That's why we recommend users not to set the gc_life_time less than 10m. However we didn't really check if the gc_life_time was set too short. In this PR, I still didn't check it.

What is changed and how it works?

This PR adds a field max-txn-time-use to the config file, tikv-client` section. Then in 2pc.go use the config value instead of a constant.

Check List

Tests

  • Unit test

Code changes

  • None of those listed

Side effects

  • None of those listed

Related changes

  • Need to update the documentation
  • Need to be included in the release note

Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Copy link
Contributor Author

/run-integration-tests tikv=release-2.0

@MyonKeminta
Copy link
Contributor Author

@disksing @zhangjinpeng1987 PTAL

@disksing
Copy link
Contributor

disksing commented Nov 7, 2018

LGTM.

@disksing
Copy link
Contributor

disksing commented Nov 7, 2018

/run-all-tests pd=release-2.0 tikv=release-2.0 tidb-test=release-2.0

@MyonKeminta
Copy link
Contributor Author

I'll also bring this thing to master.

Signed-off-by: MyonKeminta <[email protected]>
@MyonKeminta
Copy link
Contributor Author

Ah.. sorry.. I forgot to sort imports and pushed again..
/run-all-tests pd=release-2.0 tikv=release-2.0 tidb-test=release-2.0

@zz-jason
Copy link
Member

zz-jason commented Nov 7, 2018

@MyonKeminta We'd better change the master branch firstly, then cherry pick the commit to the release-2.1 and release-2.0.

@MyonKeminta
Copy link
Contributor Author

MyonKeminta commented Nov 12, 2018

Then this PR is equivalent to cherry-picking #8215 .
@zz-jason PTAL

@zz-jason zz-jason changed the title [release-2.0] store/tikv: Make maxTxnTimeUse a configurable value store/tikv: Make maxTxnTimeUse a configurable value (#8215) Nov 12, 2018
@zz-jason
Copy link
Member

lGTM

@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 12, 2018
@MyonKeminta
Copy link
Contributor Author

/run-all-tests pd=release-2.0 tikv=release-2.0 tidb-test=release-2.0

@MyonKeminta
Copy link
Contributor Author

@zz-jason Tag this PR as release blocker?

@zz-jason
Copy link
Member

@MyonKeminta No need to, we are not going to release a new version on the 2.0 branch.

@MyonKeminta
Copy link
Contributor Author

/run-all-tests pd=release-2.0 tikv=release-2.0 tidb-test=release-2.0

@zz-jason zz-jason merged commit a59be18 into pingcap:release-2.0 Nov 12, 2018
@MyonKeminta MyonKeminta deleted the misono/max-txn-time-configurable branch November 12, 2018 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants