*: add system variable to turn on/off the large transaction feature #13299
Conversation
Add two global/session system variables: - tidb_enable_large_txn - tidb_large_txn_size
Codecov Report
@@ Coverage Diff @@
## master #13299 +/- ##
==============================================
- Coverage 80.4271% 80.1371% -0.29%
==============================================
Files 469 469
Lines 113182 112179 -1003
==============================================
- Hits 91029 89897 -1132
- Misses 15207 15314 +107
- Partials 6946 6968 +22 |
|
/run-all-tests |
|
/rebuild |
| case TiDBEnableLargeTxn: | ||
| s.EnableLargeTxn = TiDBOptOn(val) | ||
| case TiDBLargeTxnSize: | ||
| s.LargeTxnSize = tidbOptInt64(val, DefTiDBLargeTxnSize) |
There was a problem hiding this comment.
maybe we need to add TiDBLargeTxnSize set value check in ValidateSetSystemVar
| req := c.buildPrewriteRequest(batch, txnSize) | ||
|
|
||
| if x := bo.ctx.Value("checkLargeTxnEnable"); x != nil { |
There was a problem hiding this comment.
This has some extra cost, can we avoid it?
There was a problem hiding this comment.
I tried failpoint but the concurrent test is a problem.
There was a problem hiding this comment.
The test doesn't make much sense.
| @@ -702,6 +702,8 @@ var defaultSysVars = []*SysVar{ | |||
| {ScopeGlobal | ScopeSession, TiDBEnableVectorizedExpression, BoolToIntStr(DefEnableVectorizedExpression)}, | |||
| {ScopeGlobal | ScopeSession, TiDBEnableFastAnalyze, BoolToIntStr(DefTiDBUseFastAnalyze)}, | |||
| {ScopeGlobal | ScopeSession, TiDBSkipIsolationLevelCheck, BoolToIntStr(DefTiDBSkipIsolationLevelCheck)}, | |||
| {ScopeGlobal | ScopeSession, TiDBEnableLargeTxn, BoolToIntStr(DefTiDBEnableLargeTxn)}, | |||
| {ScopeGlobal | ScopeSession, TiDBLargeTxnSize, strconv.Itoa(DefTiDBLargeTxnSize)}, | |||
There was a problem hiding this comment.
I think we can use a constant for it instead of a session variable.
There was a problem hiding this comment.
Constant for TiDBEnableLargeTxn and TiDBLargeTxnSize ? or for DefTiDBXXX
| @@ -702,6 +702,8 @@ var defaultSysVars = []*SysVar{ | |||
| {ScopeGlobal | ScopeSession, TiDBEnableVectorizedExpression, BoolToIntStr(DefEnableVectorizedExpression)}, | |||
| {ScopeGlobal | ScopeSession, TiDBEnableFastAnalyze, BoolToIntStr(DefTiDBUseFastAnalyze)}, | |||
| {ScopeGlobal | ScopeSession, TiDBSkipIsolationLevelCheck, BoolToIntStr(DefTiDBSkipIsolationLevelCheck)}, | |||
| {ScopeGlobal | ScopeSession, TiDBEnableLargeTxn, BoolToIntStr(DefTiDBEnableLargeTxn)}, | |||
There was a problem hiding this comment.
I think we can use a config file option, so we can deprecate it later.
|
@coocood disagree with adding session variables to turn on this new feature. |
What problem does this PR solve?
Before the new feature is stable, we must be able to turn on/off it.
What is changed and how it works?
Add two global/session system variables:
Check List
Tests
Code changes