search

ShahriyarR / MySQL-AutoXtraBackup

MySQL-AutoXtraBackup commandline tool written in Python 3 based on Percona XtraBackup
https://autoxtrabackup.azepug.az/
MIT License
139 stars 79 forks source link

Return status for inc_backup() is IGNORED !? #430

Open WernerMairl opened 3 years ago

WernerMairl commented 3 years ago

Hi you are ignoring the result (0/1) for inc_backup(), the entire autoxtrbackup run results in a false positive exitcode! That may not be the best practice :-(

https://github.com/ShahriyarR/MySQL-AutoXtraBackup/blob/72ca0a834f12331c3ed8cde17ea77a8ce573a774/master_backup_script/backuper.py#L753

https://github.com/ShahriyarR/MySQL-AutoXtraBackup/blob/72ca0a834f12331c3ed8cde17ea77a8ce573a774/master_backup_script/backuper.py#L760

impact: i got some error in the next run of autoxtrabackup during preparation.... but it was really hard to find out that the root cause was in the backup before with a false positive exit :-(

regards Werner

ShahriyarR commented 3 years ago

:( sorry for the bad experience, in fact, most of the code base has been refactored for release_v2.0. https://github.com/ShahriyarR/MySQL-AutoXtraBackup/pull/389

I have implemented docker container runs, API calls, etc. Basically, we need to release this major release with a bunch of improvements - but at the time being, I am doing so much different things that, could not wrap up with this release.
Could you please suggest a fix for this issue? - I can add this fix also to the 2.0 release. Thanks)

WernerMairl commented 3 years ago

Could you please suggest a fix for this issue?

Not as a py code because i have never used py as a developer....

What i suggest: The caller of backup_inc() should check the return code and if not 0 then maybe a runtime error can be thrown (fail fast and early).

That may have impact to some workflow, but better a runtime exception then a false positive result.

regards Werner

ShahriyarR commented 3 years ago

Indeed, proper error handling related to exit codes should be applied here as well. Thanks for the report - converting to the bug as verified.

WernerMairl commented 3 years ago

found another "worst case" scenario (same bug pattern) for full backup:

the follwoing line is not respecting the result from line 703!

https://github.com/ShahriyarR/MySQL-AutoXtraBackup/blob/72ca0a834f12331c3ed8cde17ea77a8ce573a774/master_backup_script/backuper.py#L712

basically i was able to do a full backup and a incremental backup both with false positive end status :-(

ShahriyarR commented 3 years ago

@WernerMairl As I have released version 2.0, this error should not be encountered anymore. Could you please checkout out the latest version of our tool? Thanks :)