Skip to content

importer: replace pg_dumpall with per-database pg_dump, add AWS IAM token refresh#8

Merged
brmzkw merged 12 commits into
mainfrom
jcastets/remove-pgdumpall
May 4, 2026
Merged

importer: replace pg_dumpall with per-database pg_dump, add AWS IAM token refresh#8
brmzkw merged 12 commits into
mainfrom
jcastets/remove-pgdumpall

Conversation

@brmzkw
Copy link
Copy Markdown
Collaborator

@brmzkw brmzkw commented May 3, 2026

Backup

  • Replace the single pg_dumpall call with per-database pg_dump -Fc
    dumps. Both single-database and full-cluster backups now produce the
    same layout: /00000-globals.sql + one numbered /000NN-<db>.dump
    per database.
  • Skip the globals dump when data_only=true (globals are schema-only).
  • Add exclude_databases (comma-separated) to skip databases during a
    full backup. The AWS importer defaults it to rdsadmin, an internal
    RDS system database that regular users cannot dump.

Restore

  • Replace the ambiguous create_db option with two explicit options:
    • clean: drop objects within the target database before restoring
      (--clean --if-exists). The database must already exist.
    • recreate: drop and recreate the entire database from the archive
      metadata (-C --clean --if-exists). The postgres database is
      special-cased and never dropped, mirroring pg_dumpall behaviour.
  • Default restore (no option) requires the target database to already
    exist — safe for selective restores into a live cluster.
  • Add databases (comma-separated) to restore only specific databases
    from a full backup. Globals are always restored unless no_globals=true.
  • Remove dead code: legacy globals.sql and generic .sql fallbacks.

AWS IAM token refresh

  • Token is now generated lazily via TokenProvider instead of once at
    construction time, in both the importer and the exporter.
  • Every method that opens a connection or launches a subprocess (Ping,
    emitManifest, emitRecord, listDatabases, pgRestore,
    psqlRestore) refreshes the token itself, so short-lived IAM tokens
    (~15 min) are always valid at point of use.

Test plan

  • [OK] Single-database backup and restore (default, clean, recreate)
  • [OK] Full-cluster backup and restore on a fresh cluster (recreate)
  • [OK] Full-cluster backup and restore on a populated cluster (recreate)
  • [OK] data_only and schema_only backups
  • [OK] databases=myapp restores only the selected database from a full backup
  • [OK] exclude_databases=rdsadmin on RDS — full backup completes without error
  • [OK] AWS IAM backup running longer than 15 minutes succeeds

brmzkw and others added 4 commits May 2, 2026 20:59
Instead of a single pg_dumpall run, the all-databases backup path now:
- dumps globals via pg_dumpall --globals-only → /00000-globals.sql
- lists connectable databases (datallowconn = true, excludes template0)
- dumps each database via pg_dump -Fc → /00001-<name>.dump, /00002-<name>.dump, …

Single-database and all-databases backups now share a single code path;
the single-database case filters the database list to the requested name.

The exporter's restore_globals option is renamed to no_globals (inverted
semantics): globals are restored by default and no_globals=true skips them.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pg_dumpall --globals-only produces schema-only output (roles, tablespaces),
so emitting it during a data-only backup is inconsistent. Suppress it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GenerateDBAuthToken is now called lazily via TokenProvider instead of
at construction time, so short-lived IAM tokens (~15 min) are always
fresh when they are actually used.  For the importer, the token is
refreshed at the start of Import() (covering Ping, emitManifest,
listDatabases, canReadPgAuthid) and again before each pg_dump /
pg_dumpall subprocess in emitRecord.  For the exporter, it is refreshed
before each pg_restore and psql invocation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Refreshing in emitManifest is consistent with the pattern used in
emitRecord and Ping: the token is renewed immediately before the
operation that needs it, rather than at a higher level in Import.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@brmzkw brmzkw marked this pull request as draft May 3, 2026 08:36
brmzkw and others added 6 commits May 3, 2026 10:51
The old create_db flag was ambiguous and broken in several restore
scenarios.  Replace it with two explicit options:

- clean=true: pass --clean --if-exists to pg_restore, dropping objects
  within the target database before recreating them.  The database must
  already exist.
- drop_and_recreate=true: pass -C --clean --if-exists, dropping the
  entire database and recreating it from the archive metadata.  Safe on
  both empty and populated clusters.

The default (no option) restores into an existing database without
touching anything else, which is the safest behaviour for a live cluster.
The two new options are mutually exclusive.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pg_dumpall never emits CREATE DATABASE for the postgres database because
it always exists and cannot be dropped.  Mirror this behaviour: when
drop_and_recreate=true, the postgres database is restored with
--clean --if-exists instead of -C --clean --if-exists, so a full backup
restores cleanly on both fresh and populated clusters without any special
user action.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The old single-database layout used globals.sql; the current layout
always produces 00000-globals.sql.  Since backward compatibility with
old snapshots is not a concern, drop the globals.sql check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
recreate is shorter and clearer — the drop is implied.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s and Exporter.Ping

- Add exclude_databases (comma-separated) to skip databases during a full
  backup.  The AWS importer defaults it to rdsadmin, which is an internal
  RDS system database that regular users cannot dump.
- refreshToken is now called in listDatabases and Exporter.Ping so every
  method that opens a connection manages its own token refresh, making
  future call sites safe by default.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The generic .sql fallback was a leftover from when pg_dumpall produced
all.sql.  The only .sql file in the current layout is 00000-globals.sql,
which is already handled explicitly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@brmzkw brmzkw marked this pull request as ready for review May 3, 2026 19:53
brmzkw and others added 2 commits May 3, 2026 22:13
When restoring a full backup, only .dump files whose database name
matches the comma-separated databases list are restored.  Globals
(00000-globals.sql) are always restored regardless of the filter.

The database name extraction is factored into a dumpBaseName helper,
reused by both the filter and the pg_restore target resolution.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- awsexporter: remove the initial GenerateDBAuthToken call at construction
  time; TokenProvider already refreshes the token before every connection
  and subprocess, so the upfront call was redundant.
- schemas and README: replace stale references to pg_dumpall with accurate
  descriptions reflecting the current per-database pg_dump layout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread awsimporter/importer.go
Comment thread awsexporter/schema.json
@brmzkw brmzkw merged commit fb9fdb4 into main May 4, 2026
2 checks passed
@brmzkw brmzkw deleted the jcastets/remove-pgdumpall branch May 4, 2026 09:56
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.

2 participants