fix: use TimescaleDB-safe restore procedure for backup recovery#16
Open
logicalangel wants to merge 2 commits intoPasarGuard:mainfrom
Open
fix: use TimescaleDB-safe restore procedure for backup recovery#16logicalangel wants to merge 2 commits intoPasarGuard:mainfrom
logicalangel wants to merge 2 commits intoPasarGuard:mainfrom
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Script as pasarguard.sh
participant DB as PostgreSQL/TimescaleDB
participant FS as File System
rect rgba(200, 220, 255, 0.5)
Note over Script: Unified Credential Setup
Script->>FS: Export PGPASSWORD
end
alt db_type = timescaledb
rect rgba(255, 200, 200, 0.5)
Note over Script,DB: TimescaleDB Restore Flow
Script->>DB: Terminate sessions for target DB
Script->>DB: Drop and recreate database
Script->>DB: Create timescaledb extension
Script->>DB: Run timescaledb_pre_restore()
Script->>FS: Edit dump (remove EXTENSION DDL)
Script->>DB: Restore dump as app user
alt restore fails
Script->>DB: Retry restore as postgres superuser
end
Script->>DB: Run timescaledb_post_restore()
Script->>Script: Set restore_success flag
end
else db_type = postgresql
rect rgba(200, 255, 200, 0.5)
Note over Script,DB: PostgreSQL Restore Flow
Script->>DB: Restore dump (app user, ON_ERROR_STOP=1)
alt restore fails
Script->>DB: Retry restore as postgres superuser
end
alt still fails
Script->>DB: Attempt restore into postgres DB
end
Script->>Script: Set restore_success flag
end
end
rect rgba(255, 220, 150, 0.5)
Note over Script: Cleanup & Exit
Script->>FS: Remove temp_restore_dir
alt restore_success flag
Script->>Script: Exit successfully
else
Script->>Script: Exit with failure
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pasarguard.sh`:
- Around line 2520-2528: The restore recreates the database using the backup's
variables (db_name and restore_user); change it to use the target installation's
identity by replacing the CREATE DATABASE command to use current_db_name and
current_db_user (e.g. CREATE DATABASE "$current_db_name" OWNER
"$current_db_user") so the recreated DB and owner match the local installation,
and ensure any subsequent references that write back to .env remain consistent
with current_db_name/current_db_user.
- Around line 2530-2547: The restore is undoing your pre-create and
timescaledb_pre_restore() call because the dump (created with pg_dump --clean)
contains "DROP EXTENSION IF EXISTS timescaledb;"; fix by either removing --clean
from the pg_dump invocation that produces temp_restore_dir/db_backup.sql or by
filtering extension-related DROP/CREATE lines out of the dump before piping it
into psql during the restore (the restore path that reads
temp_restore_dir/db_backup.sql and the commands around timescaledb_pre_restore()
/ "CREATE EXTENSION IF NOT EXISTS timescaledb;" should be updated accordingly to
ensure timescaledb is not dropped during restore).
- Around line 2541-2547: The psql restore commands invoked via docker exec (the
two lines using psql -U "$restore_user" -d "$db_name" <
"$temp_restore_dir/db_backup.sql" and the fallback psql -U postgres -d
"$db_name" < "$temp_restore_dir/db_backup.sql") must include ON_ERROR_STOP so
failures cause a non-zero exit and don't set restore_success=true; update both
psql invocations to pass -v ON_ERROR_STOP=1 (e.g., psql -v ON_ERROR_STOP=1 -U
"$restore_user" -d "$db_name" ...) so partial SQL failures abort the command and
the restore_success flag only becomes true on a full successful restore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pasarguard.sh`:
- Line 2507: The script currently exports PGPASSWORD="$restore_password" once
and reuses it for all subsequent psql -U postgres calls (including the
TimescaleDB admin steps and the postgres fallback), which can incorrectly use
old backup-side credentials; update each block that invokes psql -U postgres to
rebind PGPASSWORD immediately before the call using the target-side
current_db_password (e.g., set PGPASSWORD="$current_db_password" for the
TimescaleDB admin steps and postgres fallback paths), ensuring you replace any
uses of restore_password for admin/postgres operations and re-export or export
per-attempt so each psql invocation authenticates with the correct target
credentials.
- Around line 2554-2561: The fallback psql attempt may replay a dump into a
partially-applied database because the initial psql call (with ON_ERROR_STOP=1)
can leave partial state; before retrying as postgres you must reset the target
DB or skip the lower-privilege attempt. Modify the block that checks docker exec
... psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db" to, on failure,
either (a) drop and recreate "$target_db" inside the container (using the
postgres superuser) before running the fallback psql invocation, or (b) omit the
lower-privilege attempt entirely and directly run the restore as user postgres;
ensure you update the restore_success variable only after a successful full
restore and apply the same change to the second occurrence noted around lines
2575-2589.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
When restoring a backup taken with a different TimescaleDB version, the plain psql restore fails because DROP EXTENSION / CREATE EXTENSION cycles break when the shared library is already loaded with the new version. This causes the internal schemas (_timescaledb_functions, _timescaledb_catalog) to never be created, crashing the application. Fix by implementing the TimescaleDB-recommended restore procedure: 1. Terminate active connections and drop/recreate the target database 2. Create the timescaledb extension in the fresh database 3. Call timescaledb_pre_restore() to enter restore mode 4. Restore the SQL dump 5. Call timescaledb_post_restore() to finalize The plain PostgreSQL restore path remains unchanged. Closes PasarGuard/panel#229
logicalangel
force-pushed
the
fix/timescaledb-backup-restore
branch
from
d359a48 to
7c72300
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pasarguard.sh (2)
2554-2561:⚠️ Potential issue | 🟠 MajorAvoid replaying fallback into a partially restored database.
If the first restore attempt fails after partial apply, retrying immediately as
postgrescan hit duplicate-object/ownership failures. Reset DB state before fallback or restore aspostgresdirectly for this path.Minimal safer approach (single-pass superuser restore for TimescaleDB path)
- if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then - restore_success=true - else - # Fallback: try with postgres superuser - colorized_echo yellow "Trying with postgres superuser..." - if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then - restore_success=true - fi - fi + if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then + restore_success=true + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pasarguard.sh` around lines 2554 - 2561, The current restore logic retries a failed psql apply as postgres which can replay into a partially applied restore; modify the restore flow in the block around the docker exec calls (the psql invocations using variables container_name, restore_user, postgres, target_db_name and temp_restore_dir/db_backup_filtered.sql) to either 1) detect a failed initial restore and drop/recreate/clean target_db_name (or otherwise rollback partial changes) inside the container before attempting the superuser fallback, or 2) skip the non-superuser attempt and run a single-pass restore as the postgres superuser for this code path; implement one of these fixes so the fallback does not replay into a partially restored DB and only set restore_success=true after a clean, complete restore.
2507-2543:⚠️ Potential issue | 🟠 MajorUse target-side credentials for
postgresadmin operations.Line 2507 binds password context from backup-derived credentials, but the
-U postgresadmin path should be tied to target installation credentials. This can still fail on cross-server restores where source and target credentials differ.Suggested adjustment
- export PGPASSWORD="$restore_password" + local app_restore_password="$restore_password" + local admin_restore_password="${current_db_password:-$restore_password}" + export PGPASSWORD="$admin_restore_password" local restore_success=false ... - if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then + export PGPASSWORD="$app_restore_password" + if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then restore_success=true else # Fallback: try with postgres superuser colorized_echo yellow "Trying with postgres superuser..." + export PGPASSWORD="$admin_restore_password" if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then restore_success=true fi fiAlso applies to: 2557-2560
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pasarguard.sh` around lines 2507 - 2543, The script is exporting the backup's restore_password for PGPASSWORD but then runs admin operations with "psql -U postgres" (docker exec ... psql -U postgres ...) which should use the target installation's postgres admin credential; update the PGPASSWORD used for those admin docker exec calls to use the target-side admin password variable (e.g., a variable like current_postgres_password or postgres_password) with a fallback to restore_password if the target admin secret is not set, and apply the same change to the other admin blocks (the TimescaleDB pre/post restore and the later admin calls around the second block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pasarguard.sh`:
- Around line 2549-2550: The grep filter currently uses 'timescaledb' as a
prefix and removes extensions like timescaledb_toolkit; update the regex in the
grep -v -E command that writes "$temp_restore_dir/db_backup_filtered.sql" from
"$temp_restore_dir/db_backup.sql" to match only the exact extension name (e.g.,
use a word-boundary or explicit boundaries such as '\btimescaledb\b' or
'(^|[[:space:]])timescaledb([[:space:]]|$)') so DROP/CREATE EXTENSION lines for
timescaledb_toolkit are not stripped; keep redirect to 2>>"$log_file" unchanged.
---
Duplicate comments:
In `@pasarguard.sh`:
- Around line 2554-2561: The current restore logic retries a failed psql apply
as postgres which can replay into a partially applied restore; modify the
restore flow in the block around the docker exec calls (the psql invocations
using variables container_name, restore_user, postgres, target_db_name and
temp_restore_dir/db_backup_filtered.sql) to either 1) detect a failed initial
restore and drop/recreate/clean target_db_name (or otherwise rollback partial
changes) inside the container before attempting the superuser fallback, or 2)
skip the non-superuser attempt and run a single-pass restore as the postgres
superuser for this code path; implement one of these fixes so the fallback does
not replay into a partially restored DB and only set restore_success=true after
a clean, complete restore.
- Around line 2507-2543: The script is exporting the backup's restore_password
for PGPASSWORD but then runs admin operations with "psql -U postgres" (docker
exec ... psql -U postgres ...) which should use the target installation's
postgres admin credential; update the PGPASSWORD used for those admin docker
exec calls to use the target-side admin password variable (e.g., a variable like
current_postgres_password or postgres_password) with a fallback to
restore_password if the target admin secret is not set, and apply the same
change to the other admin blocks (the TimescaleDB pre/post restore and the later
admin calls around the second block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pasarguard.sh (2)
2554-2561:⚠️ Potential issue | 🟠 MajorFallback restore retries into partially-restored state.
If the first
psql -v ON_ERROR_STOP=1attempt fails, earlier SQL may already be applied. Replaying the same dump without resetting the DB can turn fallback into duplicate-object/ownership failures.Suggested fix (TimescaleDB path)
- if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then - restore_success=true - else - # Fallback: try with postgres superuser - colorized_echo yellow "Trying with postgres superuser..." - if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then - restore_success=true - fi - fi + # Restore once with postgres on a fresh DB to avoid partial-state replay issues. + if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then + restore_success=true + fi#!/bin/bash # Inspect retry flow for restore attempts without DB reset between attempts. sed -n '2548,2600p' pasarguard.shAlso applies to: 2578-2592
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pasarguard.sh` around lines 2554 - 2561, The fallback retry can apply the dump twice because partial SQL from the first failed psql run may have altered objects; before attempting the fallback that uses postgres superuser, reset the target database to a clean state by dropping and recreating "$target_db_name" inside the same container (use docker exec "$container_name" psql -U postgres -c "DROP DATABASE ...; CREATE DATABASE ...") or otherwise ensure a fresh DB, then re-run the docker exec psql restore of "$temp_restore_dir/db_backup_filtered.sql"; update the branch around the docker exec lines that reference restore_user, postgres, target_db_name, temp_restore_dir/db_backup_filtered.sql and ensure any failure logs are captured to "$log_file" and restore_success is only set true after a successful full restore.
2507-2507:⚠️ Potential issue | 🟠 MajorPass
PGPASSWORDexplicitly intodocker execcalls.
export PGPASSWORD="$restore_password"at Line 2507 is host-scoped, butpsqlruns inside the container. This makes password selection brittle (especially for-U postgrespaths) and can fail on non-trust auth setups.Suggested fix
- export PGPASSWORD="$restore_password" + local app_pg_password="$restore_password" + local admin_pg_password="${current_db_password:-$restore_password}" @@ - docker exec "$container_name" psql -U postgres -d postgres \ + docker exec -e PGPASSWORD="$admin_pg_password" "$container_name" psql -U postgres -d postgres \ @@ - if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then + if docker exec -i -e PGPASSWORD="$app_pg_password" "$container_name" psql -v ON_ERROR_STOP=1 -U "$restore_user" -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then @@ - if docker exec -i "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then + if docker exec -i -e PGPASSWORD="$admin_pg_password" "$container_name" psql -v ON_ERROR_STOP=1 -U postgres -d "$target_db_name" < "$temp_restore_dir/db_backup_filtered.sql" 2>>"$log_file"; then#!/bin/bash # Verify host-level PGPASSWORD export and docker-exec psql calls missing explicit env passing. rg -n 'export PGPASSWORD|docker exec .*psql' pasarguard.sh -C2 rg -n 'docker exec .*psql' pasarguard.sh | grep -v -- '-e PGPASSWORD='Also applies to: 2528-2534, 2554-2559, 2578-2585
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pasarguard.sh` at line 2507, The host-level export PGPASSWORD="$restore_password" is ineffective for psql commands run inside containers; update every docker exec ... psql invocation (and related functions) to pass the password explicitly via docker exec -e PGPASSWORD="$restore_password" (or the correct variable name in that scope) instead of relying on the host export; locate uses of the exported PGPASSWORD and all docker exec ... psql invocations (e.g., the blocks near the current export PGPASSWORD and the docker exec calls mentioned) and add -e PGPASSWORD="$restore_password" to each docker exec, or remove the host export if it's no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pasarguard.sh`:
- Line 2529: The psql command interpolates $target_db_name directly into SQL
causing SQL injection; change the psql invocation that builds the "SELECT
pg_terminate_backend(... WHERE datname = ...)" query to pass the shell variable
into psql via --set (e.g. --set=target_db_name="$target_db_name") and use psql
variable expansion as an SQL literal when constructing the WHERE clause (use
:'target_db_name' or quote_literal(:'target_db_name') in the SQL) so the datname
comparison is safely quoted instead of using '$target_db_name' string
interpolation.
---
Duplicate comments:
In `@pasarguard.sh`:
- Around line 2554-2561: The fallback retry can apply the dump twice because
partial SQL from the first failed psql run may have altered objects; before
attempting the fallback that uses postgres superuser, reset the target database
to a clean state by dropping and recreating "$target_db_name" inside the same
container (use docker exec "$container_name" psql -U postgres -c "DROP DATABASE
...; CREATE DATABASE ...") or otherwise ensure a fresh DB, then re-run the
docker exec psql restore of "$temp_restore_dir/db_backup_filtered.sql"; update
the branch around the docker exec lines that reference restore_user, postgres,
target_db_name, temp_restore_dir/db_backup_filtered.sql and ensure any failure
logs are captured to "$log_file" and restore_success is only set true after a
successful full restore.
- Line 2507: The host-level export PGPASSWORD="$restore_password" is ineffective
for psql commands run inside containers; update every docker exec ... psql
invocation (and related functions) to pass the password explicitly via docker
exec -e PGPASSWORD="$restore_password" (or the correct variable name in that
scope) instead of relying on the host export; locate uses of the exported
PGPASSWORD and all docker exec ... psql invocations (e.g., the blocks near the
current export PGPASSWORD and the docker exec calls mentioned) and add -e
PGPASSWORD="$restore_password" to each docker exec, or remove the host export if
it's no longer needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Drop and recreate the target database for a clean slate | ||
| colorized_echo blue "Dropping and recreating database '$target_db_name'..." | ||
| docker exec "$container_name" psql -U postgres -d postgres \ | ||
| -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '$target_db_name' AND pid <> pg_backend_pid();" \ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate raw SQL interpolation points for DB name in restore path.
rg -n "datname = '\\$target_db_name'|CREATE DATABASE .*\\$target_db_name|DROP DATABASE .*\\$target_db_name" pasarguard.sh -C1Repository: PasarGuard/scripts
Length of output: 190
🏁 Script executed:
# First, find the pasarguard.sh file
find . -name "pasarguard.sh" -type fRepository: PasarGuard/scripts
Length of output: 77
🏁 Script executed:
# Check the specific line 2529 and surrounding context
sed -n '2520,2540p' ./pasarguard.sh | cat -nRepository: PasarGuard/scripts
Length of output: 1696
🏁 Script executed:
# Check where db_name originates (from backup metadata parsing)
rg -n "db_name\s*=" ./pasarguard.sh | head -20Repository: PasarGuard/scripts
Length of output: 713
🏁 Script executed:
# Also check for other similar SQL patterns with unquoted variables
rg -n "WHERE.*= '\\\$" ./pasarguard.shRepository: PasarGuard/scripts
Length of output: 211
🏁 Script executed:
# Check the context around line 2265 to understand where db_name comes from
sed -n '2255,2275p' ./pasarguard.sh | cat -nRepository: PasarGuard/scripts
Length of output: 930
🏁 Script executed:
# Search for all patterns where variables are interpolated into SQL strings without proper quoting
rg -n "WHERE.*= '\\\$|FROM.*'\\\$|AND.*= '\\\$" ./pasarguard.shRepository: PasarGuard/scripts
Length of output: 211
🏁 Script executed:
# Also search for other datname or CREATE/DROP DATABASE patterns with unquoted variables
rg -n "datname|DROP DATABASE|CREATE DATABASE" ./pasarguard.sh -A1 -B1 | head -40Repository: PasarGuard/scripts
Length of output: 714
Fix SQL injection vulnerability in database termination query.
Line 2529 uses unquoted string interpolation ('$target_db_name') in the SQL WHERE clause, while the DROP and CREATE statements on lines 2532-2534 properly quote the same variable. Since target_db_name originates from backup metadata (untrusted input), a crafted database name with a single quote could break SQL parsing. Use psql variables for safe literal quoting:
Suggested fix
- docker exec "$container_name" psql -U postgres -d postgres \
- -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '$target_db_name' AND pid <> pg_backend_pid();" \
+ docker exec "$container_name" psql -U postgres -d postgres \
+ -v target_db_name="$target_db_name" \
+ -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = :'target_db_name' AND pid <> pg_backend_pid();" \
+ >>"$log_file" 2>&1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '$target_db_name' AND pid <> pg_backend_pid();" \ | |
| docker exec "$container_name" psql -U postgres -d postgres \ | |
| -v target_db_name="$target_db_name" \ | |
| -c "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = :'target_db_name' AND pid <> pg_backend_pid();" \ | |
| >>"$log_file" 2>&1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pasarguard.sh` at line 2529, The psql command interpolates $target_db_name
directly into SQL causing SQL injection; change the psql invocation that builds
the "SELECT pg_terminate_backend(... WHERE datname = ...)" query to pass the
shell variable into psql via --set (e.g. --set=target_db_name="$target_db_name")
and use psql variable expansion as an SQL literal when constructing the WHERE
clause (use :'target_db_name' or quote_literal(:'target_db_name') in the SQL) so
the datname comparison is safely quoted instead of using '$target_db_name'
string interpolation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Restoring a backup taken with a different TimescaleDB version causes the application to crash with:
This happens because the plain
psql < dump.sqlrestore fails when TimescaleDB versions differ between the backup source and restore target. The dump'sDROP EXTENSION/CREATE EXTENSION IF NOT EXISTS timescaledbcycle breaks because the shared library is already loaded with the new version. The internal schemas (_timescaledb_functions,_timescaledb_catalog,_timescaledb_config) never get created, and every subsequent query fails.Solution
For TimescaleDB restores specifically, implement the officially recommended restore procedure:
timescaledb_pre_restore()to put TimescaleDB into restore modetimescaledb_post_restore()to finalize (always called, even on error, to leave DB usable)The plain PostgreSQL restore path remains completely unchanged.
Testing
Tested locally with Docker (
timescale/timescaledb:latest-pg16):SELECT pg_catalog.version()works (the exact query that was crashing)Closes PasarGuard/panel#229
Summary by CodeRabbit
Bug Fixes
Refactor