| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
| Subject: | Re: Orphaned Files in PostgreSQL |
| Date: | 2026-09-23 22:10:27 |
| Message-ID: | CAN4CZFP=oPmqrNq5LU0-rs1giu6x6hjWOD7AOStz3S2JbqvhxA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello!
This issue is also related to a recent discussion about online
checksums[1] and I tried to look into possible solutions into it when
investigating that, and I agree that it should be improved.
But I think the proposed patch has some issues.
On Wed, 23 Sep 2026, Andrey Borodin <amborodin(at)acm(dot)org> wrote:
> From reading v2, I am concerned about mapped catalog rewrites.
> write_relmap_file() flushes XLOG_RELMAP_UPDATE before calling
> RelationPreserveStorage(), where the patch now records PRESERVE.
> A crash between those steps leaves the new mapping durable, but the
> creating transaction uncommitted and its manifest without PRESERVE.
It doesn't need a random crash, PITR to a VACUUM FULL
pg_class/pg_database/... with recovery_target_action = 'promote' can
crash the server / completely brick the datadir.
For example if waldump shows:
Storage 0/030380B0 PRECREATE base/5/16387
RelMap 0/03040E48 UPDATE database ...
Storage 0/03041080 PRESERVE base/5/16384 xid 664
Storage 0/030410B0 PRESERVE base/5/16387 xid 664
Transaction 0/03041140 COMMIT
Then PITR to that RelMap entry reproduces the issue.
Another recovery issue is that end of recovery reconciliation runs
after the cluster already left recovery, and it will unlink the
storage of live transactions.
For example retrying BEGIN; CREATE TABLE ... ; loops accross a pg_ctl
promote pulls the storage out from the new tables, the transactions
can still COMMIT, and then access to that table fails because there's
no storage for it.
@@ -262,6 +697,15 @@ RelationPreserveStorage(RelFileLocator rlocator,
bool atCommit)
if (RelFileLocatorEquals(rlocator, pending->rlocator)
&& pending->atCommit == atCommit)
{
+ if (!atCommit && TransactionIdIsValid(pending->createXid))
+ {
This performs IO inside a critical section and can panic the server if
that IO fails. And if that panic happens with a catalog table, the
database can't start up again, because recovery unlinks the new file.
> The patch also handles subtransactions, prepared transactions, standby
> replay, truncated manifests, and failures that leave retired manifests
> behind.
It seems to me that the standby keeps crash aborted orphan files, only
the primary deletes them properly.
> It would be useful to compare small-DDL and replay
> costs before settling on synchronous per-record writes.
Other than costs, currently postgres honors synchronous_commit = off
for storage creating DDL. With the patch, it no longer does so. That
at least needs proper documentation, but I am not so sure that this is
an actual requirement for preventing orphan files.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alberto Piai | 2026-09-23 22:19:05 | Re: Adding a stored generated column without long-lived locks |
| Previous Message | Masahiko Sawada | 2026-09-23 22:00:01 | Re: Add REPACK progress phases for logical decoding setup |