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

Prevent gRRPC's BackupStatus from canceling pending futures #745

Merged
merged 2 commits into from
May 6, 2024

Conversation

rzvoncek
Copy link
Contributor

Fixes #742.

Prior to this patch, I could even reproduce the exception from the issue in the integration tests:

[2024-04-11 13:17:02,362] INFO: Registered backup id grpc_backup_23
[2024-04-11 13:17:02,363] INFO: Recording async backup information.
[2024-04-11 13:17:02,363] ERROR: Exception in callback record_backup_info(<Future cancelled>) at /Users/zvo/github/cassandra-medusa/medusa/service/grpc/server.py:346
handle: <Handle record_backup_info(<Future cancelled>) at /Users/zvo/github/cassandra-medusa/medusa/service/grpc/server.py:346>
Traceback (most recent call last):
  File "/Users/zvo/anaconda3/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/Users/zvo/github/cassandra-medusa/medusa/service/grpc/server.py", line 349, in record_backup_info
    if future.exception():
       ^^^^^^^^^^^^^^^^^^
asyncio.exceptions.CancelledError: Removal of backup requested. Cancelling backup Name: grpc_backup_23 with done state: False

@rzvoncek rzvoncek force-pushed the radovan/grpc-backupStatus-reregister branch 2 times, most recently from 6086305 to b3d85a4 Compare April 11, 2024 13:13
@rzvoncek rzvoncek force-pushed the radovan/grpc-backupStatus-reregister branch from b3d85a4 to 4e8464d Compare April 12, 2024 08:34
@@ -99,7 +99,7 @@ def set_backup_future(backup_name, future):

# Sets the future for a backup; unknown on overall status at this point unless already existing
@staticmethod
def register_backup(backup_name, is_async):
def register_backup(backup_name, is_async, overwrite_existing=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: In which case is the overwrite_existing=True and is_async=True ? I can't find such invocation in current codebase so I was wondering is the overwrite_existing=False intended to be used only with async cases?

@smutel
Copy link

smutel commented Apr 26, 2024

Hello,

I have the same issue. Do you have any workaround to suggest me ? The backups seems completely hanged ...

Thanks.

medusa/backup_manager.py Outdated Show resolved Hide resolved
medusa/service/grpc/server.py Show resolved Hide resolved
Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

One logging is inaccurate now and I would like to understand why we're re-registering backups when checking their status.

Copy link

sonarcloud bot commented May 6, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@rzvoncek rzvoncek merged commit 1045f57 into master May 6, 2024
29 checks passed
@rzvoncek rzvoncek deleted the radovan/grpc-backupStatus-reregister branch June 12, 2024 09:49
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.

gRPC server's BackupStatus cancels AsyncBackup's future
4 participants