Aiven-Open / myhoard

MySQL Backup and Point-in-time Recovery service
Apache License 2.0
93 stars 20 forks source link

mark basebackups as broken when restore fails #145

Closed kathia-barahona closed 1 year ago

kathia-barahona commented 1 year ago

Will close PR: https://github.com/aiven/myhoard/pull/143

Following @rikonen idea about marking basebackups as broken instead of having a target time for basebackups. Will create a ticket for marking/unmarking basebackups as broken via API request.

codecov-commenter commented 1 year ago

Codecov Report

Base: 77.39% // Head: 77.36% // Decreases project coverage by -0.04% :warning:

Coverage data is based on head (33de06e) compared to base (6541bf9). Patch coverage: 67.44% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #145 +/- ## ========================================== - Coverage 77.39% 77.36% -0.04% ========================================== Files 17 17 Lines 4283 4325 +42 Branches 972 981 +9 ========================================== + Hits 3315 3346 +31 - Misses 729 735 +6 - Partials 239 244 +5 ``` | [Impacted Files](https://codecov.io/gh/aiven/myhoard/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven) | Coverage Δ | | |---|---|---| | [myhoard/backup\_stream.py](https://codecov.io/gh/aiven/myhoard/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven#diff-bXlob2FyZC9iYWNrdXBfc3RyZWFtLnB5) | `77.34% <60.00%> (-0.50%)` | :arrow_down: | | [myhoard/controller.py](https://codecov.io/gh/aiven/myhoard/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven#diff-bXlob2FyZC9jb250cm9sbGVyLnB5) | `79.47% <73.91%> (-0.15%)` | :arrow_down: | | [myhoard/basebackup\_operation.py](https://codecov.io/gh/aiven/myhoard/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven#diff-bXlob2FyZC9iYXNlYmFja3VwX29wZXJhdGlvbi5weQ==) | `89.88% <0.00%> (-0.38%)` | :arrow_down: | | [myhoard/restore\_coordinator.py](https://codecov.io/gh/aiven/myhoard/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven#diff-bXlob2FyZC9yZXN0b3JlX2Nvb3JkaW5hdG9yLnB5) | `78.57% <0.00%> (+0.45%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiven)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

kathia-barahona commented 1 year ago

Also one more question: will this play nicely with pre-existing backups in prod where they don't have a broken_at key at all in metadata?

Shouldn't be a problem. We fill the metadata of each backup based on the jsons we store per status, so even if there is no broken.json for the backup, the metadata will still have the key. https://github.com/aiven/myhoard/blob/eb829289b3292963ac1b7211a75264ddc04c0e77/myhoard/controller.py#L567 https://github.com/aiven/myhoard/blob/eb829289b3292963ac1b7211a75264ddc04c0e77/myhoard/controller.py#L594-L605

alanfranz commented 1 year ago

Shouldn't be a problem. We fill the metadata of each backup based on the jsons we store per status, so even if there is no broken.json for the backup, the metadata will still have the key.

A test for this would be great, btw!

kathia-barahona commented 1 year ago

seems one of the tests is flaky :/ need to see how to fix it.