fix: replace destructive Dagster event cleanup with safe retention + backups (#132)#150
Conversation
…backups (#132) The dagster_db_cleanup job deleted every non-error event older than 24h and cascade-deleted asset_materializations, asset_observations, asset_check_executions, and event_log_tags. Those index tables are orchestration/automation state — AutomationConditions, "latest materialization", asset checks, and lineage read from them — so cleanup made materialized assets look un-materialized and could retrigger automation. Retention (safe, by event class): - cleanup_event_logs.sh now deletes ONLY plain log-message rows (event_logs with dagster_event_type IS NULL), in bounded batches, default 30-day retention. It never touches structured events or the asset index tables. Legacy DAGSTER_EVENT_RETENTION_HOURS is still honored (now log-rows only). - dagster.yaml gains Dagster's supported `retention:` for schedule/sensor ticks (the actual high-volume tick tables), no raw SQL. Backups: - backup_dagster_postgres.sh + dagster_db_backup compose service: interval pg_dump -> gzipped timestamped dumps on a dagster_backups volume, retention pruning, optional GPG encryption (GCP PD is encrypted at rest regardless). - docker/DAGSTER_RETENTION_AND_BACKUP.md documents retention-by-class, RPO (24h) / RTO (~15m), and a tested restore drill. Config surfaced in .env.dagster.example. Tests (test_dagster_retention_cleanup.py) statically enforce the contract: cleanup never DELETEs the protected tables, deletes only NULL-type event_logs rows, scripts pass `sh -n`, dagster.yaml declares tick retention, and compose wires the backup service + volume. (Live restore drill must be run against a real PostgreSQL — cannot execute in CI.) Closes #132 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38cf3e5e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- backup_dagster_postgres.sh: dump to a temp file and check pg_dump's own exit status before gzipping. POSIX sh has no pipefail, so `pg_dump | gzip` reported gzip's status and could publish an empty/partial dump as success; pruning then ran and could rotate away good backups. Now gzip/publish/prune happen only after a verified successful dump. - docker-compose.yml: pass legacy DAGSTER_EVENT_RETENTION_HOURS through to dagster_db_cleanup so deployments that still set it keep their configured retention instead of silently jumping to the 30-day default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9321a65a9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 3. Restore the dump into the scratch DB. | ||
| # (If the file ends in .gpg, first: gpg --batch --passphrase "$PASS" -d file.sql.gz.gpg > file.sql.gz) | ||
| docker compose exec dagster_db_backup sh -c \ | ||
| 'gunzip -c /backups/<chosen>.sql.gz | psql -h dagster_postgresql -U dagster_user -d dagster_restore_test' |
There was a problem hiding this comment.
In the documented restore drill, this psql runs inside dagster_db_backup and connects over TCP to dagster_postgresql, but the compose environment only defines DAGSTER_DB_PASSWORD; the PGPASSWORD export in backup_dagster_postgres.sh is local to the backup service process and is not inherited by a new docker compose exec shell. In a non-interactive drill this will prompt/fail instead of restoring, so the command should set PGPASSWORD="$DAGSTER_DB_PASSWORD" (or use a .pgpass) before invoking psql.
Useful? React with 👍 / 👎.
|
|
||
| ```bash | ||
| docker compose stop dagster_webserver dagster_daemon dagster_user_code | ||
| gunzip -c /backups/<chosen>.sql.gz | \ |
There was a problem hiding this comment.
This production restore command is run from the host, but /backups is only mounted inside the dagster_db_backup container as a Docker named volume. Unless an operator separately bind-mounts that path on the host, gunzip cannot find the dump during an outage; pipe the dump from docker compose exec dagster_db_backup gunzip -c /backups/... or document the actual host volume path.
Useful? React with 👍 / 👎.
1 participant
Summary
The
dagster_db_cleanupjob (docker/cleanup_event_logs.sh) deleted every non-error event older than 24h and cascade-deletedasset_materializations,asset_observations,asset_check_executions, andevent_log_tags. Those index tables are orchestration/automation state —AutomationConditions, "latest materialization", asset checks, and lineage all read from them — so the cleanup made materialized assets look un-materialized, could retrigger automation, and capped incident history at ~1 day. There were also no backups.Retention — safe, by event class
event_logswheredagster_event_type IS NULL)DAGSTER_LOG_RETENTION_DAYS(30d)cleanup_event_logs.shretention:indagster.yamlcleanup_event_logs.shrewritten: deletes onlydagster_event_type IS NULLrows (the high-volumecontext.log.*lines) in bounded batches. It no longer touches the asset index tables at all — those rows never have index entries, so no state can be orphaned. LegacyDAGSTER_EVENT_RETENTION_HOURSis still honored but now affects log rows only.dagster.yaml: adds Dagster's supportedretention:block for schedule/sensor ticks (the real high-volume tick data) — no raw SQL against internals.Backups
backup_dagster_postgres.sh+dagster_db_backupcompose service: intervalpg_dump→ gzipped timestamped dumps on adagster_backupsvolume, retention pruning, optional GPG symmetric encryption. GCP persistent disks are AES-256 encrypted at rest, so the volume is encrypted regardless.docker/DAGSTER_RETENTION_AND_BACKUP.md: retention-by-class table, RPO 24h / RTO ~15m, and a copy-pasteable restore drill (restore into a scratch DB, assertasset_materializationscount survives).New knobs surfaced in
.env.dagster.example.Tests
test_dagster_retention_cleanup.pystatically enforces the safety contract (no PostgreSQL in CI):DELETE FROMany protected table; deletes onlydagster_event_type IS NULLevent_logsrows (single DELETE, no old "except errors" predicate);sh -n;dagster.yamldeclares schedule+sensor tick retention;/backupsvolume.docker compose configvalidates;ruffclean.Acceptance criteria
event_logs.dagster_event_typepredicate — Dagster OSS has no supported log-row retention API; tick retention uses the supportedretention:config)Closes #132
🤖 Generated with Claude Code