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.

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

Fix backup man future registration and BackupStatus computation of the node backup state #799

Merged
merged 5 commits into from
Aug 15, 2024

Conversation

adejanovski
Copy link
Contributor

@adejanovski adejanovski commented Aug 14, 2024

Fixes #797

Two bugs are fixed in this PR:

  • The BackupMan.register_backup() function was always registering the backup, even if it existed and overwrite was set to false due to bad code structure.
  • The state was computed without taking into account the presence of a future. In case of a restart of the grpc server with an unfinished backup job, the backup would forever be seen as in progress even though it failed.

@adejanovski adejanovski marked this pull request as draft August 14, 2024 11:39
@adejanovski adejanovski force-pushed the fix-backup-status-tracking-futures branch from 5a1a290 to 745dc60 Compare August 14, 2024 13:25
Copy link

sonarcloud bot commented Aug 14, 2024

@adejanovski adejanovski marked this pull request as ready for review August 14, 2024 16:41
@adejanovski
Copy link
Contributor Author

@rzvoncek, this is ready for review. I added some tests to check the behavior and manually checked that it works as expected in K8ssandra.
There are 2 failing tests which seem unrelated but I'll leave that up to you to decide (one seems to be a gcs communication issue and the other a webhook issue).

@rzvoncek rzvoncek merged commit 6ff14ee into master Aug 15, 2024
29 checks passed
@rzvoncek
Copy link
Contributor

I've retriggered the tests and they both passed.

rzvoncek pushed a commit that referenced this pull request Aug 15, 2024
…e node backup state (#799)

* Fix backup man future registration and BackupStatus computation of the node backup state

* Attempt at using docker compose v2

* Handle future result

* add logging to understand how the future behaves

* Fix exception using a wrong method on futures
SoaigM pushed a commit to SoaigM/cassandra-medusa that referenced this pull request Aug 23, 2024
…e node backup state (thelastpickle#799)

* Fix backup man future registration and BackupStatus computation of the node backup state

* Attempt at using docker compose v2

* Handle future result

* add logging to understand how the future behaves

* Fix exception using a wrong method on futures
SoaigM pushed a commit to SoaigM/cassandra-medusa that referenced this pull request Aug 23, 2024
…e node backup state (thelastpickle#799)

* Fix backup man future registration and BackupStatus computation of the node backup state

* Attempt at using docker compose v2

* Handle future result

* add logging to understand how the future behaves

* Fix exception using a wrong method on futures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backup tracking is broken in the grpc server
2 participants