-
Couldn't load subscription status.
- Fork 9
Remove grand-central tables before restoring a full snapshot #782
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
base: master
Are you sure you want to change the base?
Conversation
b4dd6ae to
a0eeecb
Compare
crate/operator/restore_backup.py
Outdated
| partitions, | ||
| sections, | ||
| ) | ||
| except kopf.PermanentError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice strategy to ensure a consistent state. But I guess any exception should call the restore_tables function, right? If any other error that you have not captured and dealt with raises an exception it should still restore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it to Exception to catch everything
|
@plaharanne I am wondering if Grand Central is being turned off during this process. I hope so because I think it is necessary, otherwise it may crash or recreate the tables. I searched a bit but didn't find anywhere whether GC is shutdown during backup restoration |
fix uppercase in test add debug logs change query to get gc tables debug on dev fix table name in query
7162bb1 to
42d34e2
Compare
| If the snapshot contains grand-central tables, rename them if they exist | ||
| in the cluster in order to recreate the new ones from the snapshot. | ||
| """ | ||
| self.gc_tables_renamed = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be set to True only if any tables were actually renamed? otherwise LGTM! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, you're right, actually I was thinking about how I could also pause grand central only if those tables are involved.
Summary of changes
If the type of restore is
allortableswith gc tables in the list, check if the snapshot contains grand central tables and temporary rename them before executing theRESTOREquery.When the snapshot is restored, either drop the old gc tables or rename them back if it failed.
Checklist
CHANGES.rst