[WIP] Set tidb_gc_life_time by sysvar #3922
Conversation
shonge
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3922 +/- ##
==========================================
- Coverage 67.88% 62.41% -5.47%
==========================================
Files 175 171 -4
Lines 18618 18133 -485
==========================================
- Hits 12638 11318 -1320
- Misses 4879 5717 +838
+ Partials 1101 1098 -3
|
shonge
changed the title
Codecov Report
@@ Coverage Diff @@
## master #3922 +/- ##
==========================================
+ Coverage 67.79% 68.72% +0.92%
==========================================
Files 177 177
Lines 18840 18866 +26
==========================================
+ Hits 12773 12965 +192
+ Misses 4945 4772 -173
- Partials 1122 1129 +7
|
shonge
force-pushed
the
update_set_tikv_gc_life_time
branch
from
4df7e17
to
b056113
Compare
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
|
/test pull-e2e-kind-serial |
|
|
||
| func (bo *GenericOptions) HasTiDBGCLifeTime(ctx context.Context, db *sql.DB) (bool, error) { | ||
| var tikvGCTime string | ||
| sql := fmt.Sprintln("SHOW GLOBAL VARIABLES LIKE ?") |
There was a problem hiding this comment.
no need to use fmt.Sprintln here, right?
| if err != nil { | ||
| return false, fmt.Errorf("query cluster %s %s failed, sql: %s, err: %v", bo, constants.TidbGCVariable, sql, err) | ||
| } | ||
| if len(tikvGCTime) > 0 { |
There was a problem hiding this comment.
you may use sql.NullString for judgment here?
| if err != nil { | ||
| return tikvGCTime, fmt.Errorf("query cluster %s %s failed, sql: %s, err: %v", bo, constants.TikvGCVariable, sql, err) | ||
| return tikvGCTime, fmt.Errorf("query cluster %s %s failed, sql: %s, err: %v", bo, constants.TidbGCVariable, sql, err) |
There was a problem hiding this comment.
| return tikvGCTime, fmt.Errorf("query cluster %s %s failed, sql: %s, err: %v", bo, constants.TidbGCVariable, sql, err) | |
| return tikvGCTime, fmt.Errorf("query cluster %s %s failed, sql: %s, err: %v", bo, gcVariable, sql, err) |
| if err != nil { | ||
| return fmt.Errorf("set cluster %s %s failed, sql: %s, err: %v", bo, constants.TikvGCVariable, sql, err) | ||
| return fmt.Errorf("set cluster %s %s failed, sql: %s, err: %v", bo, constants.TidbGCVariable, sql, err) |
There was a problem hiding this comment.
| return fmt.Errorf("set cluster %s %s failed, sql: %s, err: %v", bo, constants.TidbGCVariable, sql, err) | |
| return fmt.Errorf("set cluster %s %s failed, sql: %s, err: %v", bo, gcVariable, sql, err) |
|
@shonge Could you please add the test result for the below cases:
|
DanielZhangQD
changed the title
|
@shonge: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What problem does this PR solve?
Close #3868
Close #3649
Dependency #3900
What is changed and how does it work?
Add
TidbGCVariable = "tidb_gc_life_time"and moveTikvGCVariable = "tikv_gc_life_time"toLegacyTikvGCVariable = "tikv_gc_life_time".Use
SHOW GLOBAL VARIABLES LIKE tidb_gc_life_timeto check if tidb supporttidb_gc_life_time.If support ,we use 'SET GLOBAL tidb_gc_life_time= 72h' to set GC life time, else using
update mysql.tidb set variable_value = 72h where variable_name = tikv_gc_life_time.Code changes
Tests
Side effects
Related changes
Release Notes
Please refer to Release Notes Language Style Guide before writing the release note.