lisk_snapshot.sh update take2 #73
Conversation
### lisk_snapshot.sh changelog - Change value of failed snapshot in progress from 24 hours to 30 minutes. - Added dropdb on target database before exit - Added missing test in waiting loop for stalled/failure detection - Added -v switch ( forced full vacuum ) to eliminate the need for lot of freespace during process. - Added -m params to allow custom delay between vacuum. (Instead of default 5 minutes) - Added spacing in usage menu - Modified end of script cleanup section. - Move stall values to minutes and defined as variables - Added date to start of each output line.
|
Now that basic script is updated, I started troubleshooting the last implematation for reccuring execution each 3 minutes for better cronjob efficiency. Using the PID as a lock file work only in command line. When it's run as cronjob the test never detect the already running instance. (issue) I will replace the PID test by a lock file (like initialy proposed) during the week. The 2nd advantage of a lock file is the fact that it can be set at start and remove at the end without the possibility of a period of time the script is running but not the node instance. |
scripts/lisk_snapshot.sh
Outdated
| fi | ||
|
|
||
| MINUTES=$(( MINUTES + 1 )) | ||
| if [ "$PGSQL_VACUUM" == "Y" ] 2> /dev/null; then |
There was a problem hiding this comment.
We dont need the flag, we should just force vacuuming mem_rounds on all deployments for consistency purposes. Adding this complexity creates more troubleshooting hurdles that are unneccessary
There was a problem hiding this comment.
I'll remove the -v switch
There was a problem hiding this comment.
And by the way, the switch is there tp allow poeple to skip the functionnality. Can I remember you that you said yourself in the chat when I proposed this modification: No, I don't have the problem myself so I don't need this functionnaility.
scripts/lisk_snapshot.sh
Outdated
| done | ||
| echo -e '\nSnapshot verification process completed at '"$(date)" | ||
| echo -e "\n$( date +'%Y-%m-%d %H:%M:%S' ) Snapshot verification process completed" |
There was a problem hiding this comment.
Please turn this date command into a callable function to improve readability
There was a problem hiding this comment.
Aceept the pull request and modify it later if it's so important to you. I won.'t modify.
There was a problem hiding this comment.
im asking you to modify it to meet our standards, this is the purpose of a code review process.
scripts/lisk_snapshot.sh
Outdated
| esac | ||
| done | ||
| } | ||
|
|
||
| usage() { | ||
| echo "Usage: $0 [-t <snapshot.json>] [-s <config.json>] [-b <backup directory>] [-d <days to keep>] [-r <round>] [-g]" | ||
| echo -e "\nUsage: $0 [-t <snapshot.json>] [-s <config.json>] [-b <backup directory>] [-d <days to keep>] [-r <round>] [-g] [-v] [-m <vacuum delay>]\n" |
There was a problem hiding this comment.
Remove this flag and just set vaccuum to always on using a hardcoded cycle of 15-30 minutes or so.
There was a problem hiding this comment.
-v is ok to remove but -m will stay with default value 3
There was a problem hiding this comment.
We do not need a flag to adjust it, we can establish standards for all systems instead.
|
Removed -v |
|
Attacking lock file implementation and initial testing tonight |
|
I just understood why there is so much double quotes in the script |
|
early testing working great so far, will report back tommorow for final confirmation |
|
everything is working fine. (3 auto-snap using lock file without bug) no more commit planned on this pull request. |
| } | ||
|
|
||
| parse_option "$@" | ||
| now() { |
lisk_snapshot.sh changelog