-
Notifications
You must be signed in to change notification settings - Fork 1
[DPE-7584] Recreate temp tablespace on reboot #11
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
Conversation
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (46.07%) is below the target coverage (70.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## 16/edge #11 +/- ##
===========================================
+ Coverage 45.45% 46.07% +0.62%
===========================================
Files 4 4
Lines 792 803 +11
Branches 92 94 +2
===========================================
+ Hits 360 370 +10
Misses 414 414
- Partials 18 19 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
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.
LGTM in general. One request to avoid data loose by bad luck => someone is fast enough to inject some data into tmp => we drop it thinking it is empty...
): | ||
change_owner(temp_location) | ||
os.chmod(temp_location, POSTGRESQL_STORAGE_PERMISSIONS) | ||
cursor.execute("DROP TABLESPACE IF EXISTS temp;") |
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.
Can we avoid blind DROPing and rename it?
Better safe then sorry.
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.
Sure. I'll make that change.
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.
Updated on ece989a.
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
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.
LGTM, please add INFO message for garbage collection.
cursor.execute("SELECT TRUE FROM pg_tablespace WHERE spcname='temp';") | ||
if cursor.fetchone() is not None: | ||
new_name = ( | ||
f"temp_{datetime.now(timezone.utc).strftime('%Y_%m_%d_%H_%M_%S')}" |
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.
Please consider to use temp_20250915223112 format. Unix oldschool story.
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.
Updated on 8a6a7b7.
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Issue
Only changing the permission of the temp storage is not enough after a reboot.
This is a follow-up from #10, which didn't solve the original issue.
Solution
Delete the temp tablespace and recreate it after a reboot.
The test using the VM charm was added at canonical/postgresql-operator#1156.
Checklist