Nightly backups - #2791
Conversation
📝 WalkthroughWalkthroughThe PR adds automated backup storage and processing, binary PostgreSQL COPY input/output support, WAL snapshot utilities, S3 upload enhancements, schema conversion helpers, storage sweeper changes, and small utility, lint, and UI updates. ChangesJDBC COPY and WAL replication
S3 backup and upload infrastructure
Schema definition planning
Storage sweeper eligibility
Utilities and interface updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Process
participant Snapshot
participant Triples
participant Upload
participant S3
participant Jobs
Process->>Snapshot: create snapshot connections
Process->>Triples: stream ordered triples
Triples->>Upload: enqueue triples
Upload->>S3: upload compressed streams
Upload->>Jobs: update progress
S3-->>Process: completed streams
Process->>Jobs: record completed backup
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
View Vercel preview at instant-www-js-backups-only-jsv.vercel.app. |
| return ( | ||
| <a | ||
| className="h-full w-full pl-2 align-middle text-xs font-bold underline hover:text-black dark:hover:text-white" | ||
| className="flex h-full w-full items-center pl-2 text-xs font-bold underline hover:text-black dark:hover:text-white" |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
server/src/instant/model/schema.clj (1)
226-271: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd round-trip coverage for schema conversion.
Test blob metadata, link cardinality/reverse uniqueness, cascade deletion, required fields, and checked data types through
schema->defs→defs->schema.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/instant/model/schema.clj` around lines 226 - 271, Add round-trip tests covering schema->defs and defs->schema, verifying blob metadata, link cardinality and reverse uniqueness, forward and reverse cascade deletion, required fields, and checked data types. Use representative schemas that preserve these attributes through both conversions and assert the reconstructed schema matches the original semantics.server/test/instant/storage/sweeper_test.clj (1)
131-133: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the default grace period.
All changed tests override the interval to
"0 days", so none verifies that recently created sweep rows remain protected by the production 32-day grace period. Add a focused test using the default interval, while retaining these overrides for throughput and loop-behavior tests.Also applies to: 163-165, 192-204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/test/instant/storage/sweeper_test.clj` around lines 131 - 133, Add a focused test around sweeper/handle-sweep! that does not redefine sweeper/storage-sweeper-grace-period-interval, and verify recently created sweep rows remain protected by the default 32-day grace period. Retain the existing "0 days" overrides in the throughput and loop-behavior tests at the referenced sections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/instant/model/schema.clj`:
- Around line 508-514: Update every caller of schema-model/plan! to use its new
two-argument signature, passing the appropriate client-defs definitions as the
second argument alongside the existing options map. Preserve the existing app-id
and other options, and ensure each caller supplies the schema definitions
intended for diffing.
In `@server/src/instant/storage/sweeper.clj`:
- Around line 135-139: The warning path invoking warn-too-many-loops! must
provide the attribute keys consumed by span-attrs: rename :current-loop to
:loops and add :limit mapped to max-loops, while preserving the existing
start-ms and app-id attributes.
---
Nitpick comments:
In `@server/src/instant/model/schema.clj`:
- Around line 226-271: Add round-trip tests covering schema->defs and
defs->schema, verifying blob metadata, link cardinality and reverse uniqueness,
forward and reverse cascade deletion, required fields, and checked data types.
Use representative schemas that preserve these attributes through both
conversions and assert the reconstructed schema matches the original semantics.
In `@server/test/instant/storage/sweeper_test.clj`:
- Around line 131-133: Add a focused test around sweeper/handle-sweep! that does
not redefine sweeper/storage-sweeper-grace-period-interval, and verify recently
created sweep rows remain protected by the default 32-day grace period. Retain
the existing "0 days" overrides in the throughput and loop-behavior tests at the
referenced sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1b5053ea-23ad-4cd9-bce7-89f124f76871
📒 Files selected for processing (17)
client/packages/components/src/components/explorer/inner-explorer.tsxserver/.clj-kondo/config.ednserver/.clj-kondo/hooks/copy.cljserver/deps.ednserver/resources/migrations/118_backups.down.sqlserver/resources/migrations/118_backups.up.sqlserver/src/instant/config.cljserver/src/instant/jdbc/copy.cljserver/src/instant/jdbc/wal.cljserver/src/instant/model/schema.cljserver/src/instant/storage/s3.cljserver/src/instant/storage/sweeper.cljserver/src/instant/system_catalog.cljserver/src/instant/util/crypt.cljserver/src/instant/util/json.cljserver/src/instant/util/s3.cljserver/test/instant/storage/sweeper_test.clj
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/instant/backup.clj`:
- Around line 80-85: Update the sql/do-execute! call in mark-backup-completed!
to use the operation keyword specific to completing a backup rather than
::update-backup-progress!. Preserve the existing connection, parameters, and SQL
behavior.
- Around line 648-654: Update the abort handling around start-upload-process to
track the active local streams, then invoke force-close on each stream during
abort before or alongside cancelling upload-process. Ensure this closes each
stream’s pipe-out/zstd/generator chain and cancels its in-flight per-etype S3
upload, while preserving the existing cancellation and query connection cleanup.
- Around line 600-610: Update the flush-stream worker loop around
complete-streams so each item’s :finished-promise is delivered in a finally
block, including when completion throws. Also update wait-for-finish to close
query-conn* and perform all existing cleanup in a finally path regardless of
worker success or failure, while preserving exception propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d9e7f76d-596f-402b-afa9-4e1a70dc78c2
📒 Files selected for processing (3)
server/src/instant/backup.cljserver/src/instant/jdbc/wal.cljserver/test/instant/jdbc/copy_test.clj
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/instant/jdbc/wal.clj
| (.multipartEnabled true) | ||
| ;; Backups can keep an upload stream open for long | ||
| ;; stretches of time without producing bytes, do | ||
| ;; what we can to prevent the connection from closing |
There was a problem hiding this comment.
I tested this out and I was able to keep the upload open for over an hour between writes without any issues.
| create table app_backups( | ||
| id uuid primary key, | ||
| app_id uuid not null references apps(id) on delete cascade, | ||
| isn isn, |
There was a problem hiding this comment.
oo til isn can be a data type
| columns)] | ||
| (if row-fn | ||
| (case column-count | ||
| 0 (construct-row-fn-0 row-fn field-decoders) |
| [:>= :t.app_id [:inline (uuid-util/coerce start-app-id)]])] | ||
| :order-by [[:t.app_id] [:t.entity_id] [:t.attr_id]]}) | ||
| _ (assert (= 1 (count select)) "The select query cannot have parameters") | ||
| q (format "/*+ IndexScan(t triples_pkey) */ COPY (%s) to stdout with (format binary)" |
stopachka
left a comment
There was a problem hiding this comment.
Beautiful work @dwwoelfel !


Creates a process to do nightly backups for all (non-ephemeral) apps with at least 1 triple.
This PR only sets up the process to create the backups. I'll run it manually to test that everything performs like we expect.
I will follow-up with PRs that will automate the process, add UI to the dashboard for downloading backups, and add support for restoring a backup.
To run a backup, we take a replication snapshot and do a single copy command on the
triplestable ordered by primary key. We build up a list of entities and directly stream them to s3 in the background. Each entity gets its ownjsonlfile in s3. When we've collected all of the triples for an app, we upload the app's schema, rules, webhooks, and email templates. Then we write a row into theapp_backupstable.We keep track of the last app we've completed. If the backup fails, we can restart from that app so that we don't have to start from scratch each time.
We store the isn of the snapshot in the
app_backupstable. Theoretically, we could apply the changes from thehistorytable to a backup to support point-in-time recovery.The ultimate bottleneck is how fast we can run
copy, so the process is optimized to avoid blocking the copy command (but there is back-pressure to prevent memory usage from spiking if the s3 upload stalls for example).If I find that the load on the db is too high or if the process is too slow, my plan B is to spin up a clone each time we do a backup. Then we can process multiple apps in parallel against the clone and shut it down when we're done.
Restoring $files
I updated the files sweeper to delay sweeping for 32 days. If a user restores a backup, then we can still restore files even if they've been deleted.
Backups are also auto-removed from s3 after 32 days (via a lifecycle rule on the
expire=truetag).Deploy plan
instant.backup/process. Ensure that everything backs up properly and that the db load is fine.