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.

Sign up for GitHub

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

Jump to bottom

add tikvGCLifeTime option #835

Merged
merged 10 commits into from
Aug 29, 2019
Merged

add tikvGCLifeTime option #835

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
78cd587
add tikvGCLifeTime option
weekface Aug 27, 2019
7b73a31
Merge branch 'master' into tikv-gc-time
weekface Aug 28, 2019
553031d
Update charts/tidb-backup/values.yaml
weekface Aug 29, 2019
bf55134
Update charts/tidb-cluster/values.yaml
weekface Aug 29, 2019
fc21aa7
Merge branch 'master' into tikv-gc-time
weekface Aug 29, 2019
35436f2
set default tikv gc time to 720h
weekface Aug 29, 2019
4019b27
Merge branch 'tikv-gc-time' of https://github.com/weekface/tidb-opera…
weekface Aug 29, 2019
5ef048b
Merge branch 'master' into tikv-gc-time
tennix Aug 29, 2019
4f43946
address comment
weekface Aug 29, 2019
e687821
Merge branch 'tikv-gc-time' of https://github.com/weekface/tidb-opera…
weekface Aug 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions charts/tidb-backup/templates/scripts/_start_backup.sh.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ fi
gc_life_time=`/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "select variable_value from mysql.tidb where variable_name='tikv_gc_life_time';"`
echo "Old TiKV GC life time is ${gc_life_time}"

echo "Increase TiKV GC life time to 3h"
/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "update mysql.tidb set variable_value='3h' where variable_name='tikv_gc_life_time';"
echo "Increase TiKV GC life time to {{ .Values.tikvGCLifeTime }}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "Increase TiKV GC life time to {{ .Values.tikvGCLifeTime }}"
echo "Increase TiKV GC life time to {{ .Values.tikvGCLifeTime | default 720 }}"

Copy link
Member

Choose a reason for hiding this comment

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

When adding new variable to chart, always remember to set a default in case of users not providing the variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "update mysql.tidb set variable_value='{{ .Values.tikvGCLifeTime }}' where variable_name='tikv_gc_life_time';"
/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "select variable_name,variable_value from mysql.tidb where variable_name='tikv_gc_life_time';"

if [ -n "{{ .Values.initialCommitTs }}" ];
Expand Down
3 changes: 3 additions & 0 deletions charts/tidb-backup/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ backupOptions: "--verbose=3"
initialCommitTs: ""
# restoreOptions is the options of loader https://www.pingcap.com/docs-cn/tools/loader/
restoreOptions: "-t 16"
# The time limit during which data is retained for each GC when backup, in the format of Go Duration.
# When a GC happens, the current time minus this value is the safe point.
tikvGCLifeTime: 3h
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would better to use a long gc life time that impossible to reach as the default, e.g. 720h, which effectively "disable" the GC when backup in progress.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think this should not even be an option: the controller should automatically hold open the GC. Here we can have a timeout option to timeout the backup if it does not complete and document that it helps limit performance degradation during backup due to maintaining the GC. TiDB is supposed to get a backup lock feature eventually.

weekface marked this conversation as resolved.
Show resolved Hide resolved

# By default, the backup/restore uses PV to store/load backup data
# You can choose to store/load backup data to/from gcp, ceph or s3 bucket by enabling the following corresponding section:
Expand Down
4 changes: 2 additions & 2 deletions charts/tidb-cluster/templates/scripts/_start_scheduled_backup.sh.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ fi
gc_life_time=`/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "select variable_value from mysql.tidb where variable_name='tikv_gc_life_time';"`
echo "Old TiKV GC life time is ${gc_life_time}"

echo "Increase TiKV GC life time to 3h"
/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "update mysql.tidb set variable_value='3h' where variable_name='tikv_gc_life_time';"
echo "Increase TiKV GC life time to {{ .Values.scheduledBackup.tikvGCLifeTime }}"
/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "update mysql.tidb set variable_value='{{ .Values.scheduledBackup.tikvGCLifeTime }}' where variable_name='tikv_gc_life_time';"
/usr/bin/mysql -h${host} -P4000 -u${TIDB_USER} ${password_str} -Nse "select variable_name,variable_value from mysql.tidb where variable_name='tikv_gc_life_time';"

/mydumper \
Expand Down
3 changes: 3 additions & 0 deletions charts/tidb-cluster/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,9 @@ scheduledBackup:
startingDeadlineSeconds: 3600
# https://github.com/maxbube/mydumper/blob/master/docs/mydumper_usage.rst#options
options: "--verbose=3"
# The time limit during which data is retained for each GC when backup, in the format of Go Duration.
# When a GC happens, the current time minus this value is the safe point.
tikvGCLifeTime: 3h
weekface marked this conversation as resolved.
Show resolved Hide resolved
# secretName is the name of the secret which stores user and password used for backup
# Note: you must give the user enough privilege to do the backup
# you can create the secret by:
Expand Down