[WIP] Implement "pg_restore --data-only --clean" as a way to skip WAL

Started by Dimitrios Apostolouabout 1 year ago7 messageshackers
Jump to latest

Hello list,

I implemented --clean support for --data-only, in order to avoid logging
to the WAL while populating the database. The attached patch issues a
TRUNCATE before COPY on each worker process, and provides a significant
speed advantage if the cluster is configure with wal_level=minimal.

It also provides a safer way to load the database, as avoiding WAL logging
also avoids potential and painful ENOSPACE on the WAL partition as I
experienced in [1]/messages/by-id/076464ad-3d70-dd25-9e8f-e84f27decfba@gmx.net. In other words it makes things much better for my use
case.

[1]: /messages/by-id/076464ad-3d70-dd25-9e8f-e84f27decfba@gmx.net

But it has some rough edges. I would appreciate guidance and feedback.

* When the table-to-be-TRUNCATEd is referenced as foreign key from other
table, the whole transaction fails with:

ERROR: cannot truncate a table referenced in a foreign key constraint

1. As a first step, when TRUNCATE fails I want to try a DELETE FROM
instead, which has more chances of succeeding, and continuing with
the COPY. How to detect the failure of ahprintf("TRUNCATE") and do
the alternative without failing the whole transaction?

2. Why doesn't --disable-triggers help?
To test this, I have manually issued

ALTER TABLE x DISABLE TRIGGER ALL

to every table and issued manual TRUNCATE still fails. Shouldn't
postgres skip the referential integrity checks?

3. In my tests, all my tables start empty since I have just created the
schema. Then pg_restore --data-only --clean first populates
the /referencing/ tables, which is allowed because of disabled
triggers, and then it tries to load the /referenced/ table.

At this point the referential integrity is already broken. Getting an
error when TRUNCATing the empty /referenced/ table doesn't make
sense.

What do you think?

Thank you in advance,
Dimitris

Attachments:

v1-0001-pg_restore-clean-data-only.patchtext/x-patch; name=v1-0001-pg_restore-clean-data-only.patchDownload+16-5
In reply to: Dimitrios Apostolou (#1)
[WIP PATCH v2] Implement "pg_restore --data-only --clean" as a way to skip WAL

On Mon, 14 Apr 2025, Dimitrios Apostolou wrote:

Hello list,

I implemented --clean support for --data-only, in order to avoid logging to
the WAL while populating the database. The attached patch issues a TRUNCATE
before COPY on each worker process, and provides a significant speed
advantage if the cluster is configure with wal_level=minimal.

It also provides a safer way to load the database, as avoiding WAL logging
also avoids potential and painful ENOSPACE on the WAL partition as I
experienced in [1]. In other words it makes things much better for my use
case.

[1] /messages/by-id/076464ad-3d70-dd25-9e8f-e84f27decfba@gmx.net

Rebased and attached v2 of the patch.

It needed some adjustments for the new flags --with-schema and
--with-data.

I have used this patch several times to pg_restore terabytes of tables
without logging through the WAL, and it performs great.

But it has some rough edges. I would appreciate guidance and feedback.

The rough edges remain: TRUNCATE fails if there are foreign keys. So if
you try pg_restore --data-only --clean to a table referenced via foreign
keys, the patch will not work, as mentioned below.

* When the table-to-be-TRUNCATEd is referenced as foreign key from other
table, the whole transaction fails with:

ERROR: cannot truncate a table referenced in a foreign key constraint

1. As a first step, when TRUNCATE fails I want to try a DELETE FROM
instead, which has more chances of succeeding, and continuing with
the COPY. How to detect the failure of ahprintf("TRUNCATE") and do
the alternative without failing the whole transaction?

2. Why doesn't --disable-triggers help?
To test this, I have manually issued

ALTER TABLE x DISABLE TRIGGER ALL

to every table and issued manual TRUNCATE still fails. Shouldn't
postgres skip the referential integrity checks?

3. In my tests, all my tables start empty since I have just created the
schema. Then pg_restore --data-only --clean first populates
the /referencing/ tables, which is allowed because of disabled
triggers, and then it tries to load the /referenced/ table.

At this point the referential integrity is already broken. Getting an
error when TRUNCATing the empty /referenced/ table doesn't make
sense.

So is there a way to turn off the referential checks for a TRUNCATE?
Do you have any other feedback for this patch?

Thanks,
Dimitris

Attachments:

v2-0001-pg_restore-clean-data-only.patchtext/x-patch; name=v2-0001-pg_restore-clean-data-only.patchDownload+26-7
In reply to: Dimitrios Apostolou (#2)
Re: [WIP PATCH v2] Implement "pg_restore --data-only --clean" as a way to skip WAL

I wonder about the following in pg_restore.c.
Right now my implementation covers only parallel restore.
In the case of non-parallel restore, I want to make the behaviour
similar, i.e. each worker to issue a TRUNCATE before COPY starts.
But then the StartTransaction() doesn't make sense, as everything might
already be in a transaction because of --single-transaction.
Should I completely skip StartTransaction() if !is_parallel?

-                                       use_truncate = is_parallel && te->created &&
+                                       use_truncate = is_parallel &&
+                                               (te->created || (ropt->dumpData && ropt->clean)) &&
                                                 !is_load_via_partition_root(te);
                                         if (use_truncate)
                                         {
+                                               pg_log_debug("BEGIN transaction and TRUNCATE table \"%s.%s\"",
+                                                                        te->namespace, te->tag);
+
                                                 /*
                                                  * Parallel restore is always talking directly to a
                                                  * server, so no need to see if we should issue BEGIN.
                                                  */
                                                 StartTransaction(&AH->public);

/*
* Issue TRUNCATE with ONLY so that child tables are
* not wiped.
*/
ahprintf(AH, "TRUNCATE TABLE ONLY %s;\n\n",
fmtQualifiedId(te->namespace, te->tag));

#4Greg Sabino Mullane
greg@turnstep.com
In reply to: Dimitrios Apostolou (#2)
Re: [WIP PATCH v2] Implement "pg_restore --data-only --clean" as a way to skip WAL

I think the overall idea is sound. But we need a better solution for the
truncate fk failure. Can we introspect somehow and do a truncate or do a
delete as necessary? I don't like the idea of simply ignoring the
constraint, or of throwing an error.

--
Cheers,
Greg

--
Crunchy Data - https://www.crunchydata.com
Enterprise Postgres Software Products & Tech Support

In reply to: Dimitrios Apostolou (#3)
[WIP PATCH v3] Implement "pg_restore --data-only --clean" as a way to skip WAL

Attaching v3 of the patch, together with a new test file that tests
previously untested flags of pg_restore.

Added to July's commitfest:
https://commitfest.postgresql.org/patch/5821/

On Thu, 12 Jun 2025, Dimitrios Apostolou wrote:

I wonder about the following in pg_restore.c.
Right now my implementation covers only parallel restore.
In the case of non-parallel restore, I want to make the behaviour similar,
i.e. each worker to issue a TRUNCATE before COPY starts.
But then the StartTransaction() doesn't make sense, as everything might
already be in a transaction because of --single-transaction.
Should I completely skip StartTransaction() if !is_parallel?

In the end, I followed the same code path for both parallel and
non-parallel pg_restore. It works even with --single-transaction, where
apparently there are several nested transactions.

The tests pass, so I assume it's OK to start a subtransaction for each
table restoration inside the global single-transaction.

Dimitris

In reply to: Dimitrios Apostolou (#5)
Re: [WIP PATCH v3] Implement "pg_restore --data-only --clean" as a way to skip WAL

Attaching files now...

On Mon, 16 Jun 2025, Dimitrios Apostolou wrote:

Show quoted text

Attaching v3 of the patch, together with a new test file that tests
previously untested flags of pg_restore.

Added to July's commitfest:
https://commitfest.postgresql.org/patch/5821/

On Thu, 12 Jun 2025, Dimitrios Apostolou wrote:

I wonder about the following in pg_restore.c.
Right now my implementation covers only parallel restore.
In the case of non-parallel restore, I want to make the behaviour similar,
i.e. each worker to issue a TRUNCATE before COPY starts.
But then the StartTransaction() doesn't make sense, as everything might
already be in a transaction because of --single-transaction.
Should I completely skip StartTransaction() if !is_parallel?

In the end, I followed the same code path for both parallel and non-parallel
pg_restore. It works even with --single-transaction, where apparently there
are several nested transactions.

The tests pass, so I assume it's OK to start a subtransaction for each table
restoration inside the global single-transaction.

Dimitris

Attachments:

v3-0001-pg_restore-clean-data-only.patchtext/x-patch; name=v3-0001-pg_restore-clean-data-only.patchDownload+37-13
v3-0002-Add-new-test-file-with-pg_restore-test-cases.patchtext/x-patch; name=v3-0002-Add-new-test-file-with-pg_restore-test-cases.patchDownload+150-1
In reply to: Greg Sabino Mullane (#4)
Re: [WIP PATCH v2] Implement "pg_restore --data-only --clean" as a way to skip WAL

On Friday 2025-06-13 03:42, Greg Sabino Mullane wrote:

I think the overall idea is sound. But we need a better solution for the truncate fk failure. Can we introspect somehow and do a truncate or do a delete as necessary? I don't like the idea of simply ignoring the constraint, or of throwing an error.

Sorry I haven't answered, but I don't really know how to catch the error
of TRUNCATE in the C code and do a DELETE instead. I was hoping someone
would chime in and suggest an approach.

Furthermore, silently replacing the TRUNCATE with DELETE will negate all
the performance benefits we are aiming for with this patch (avoiding WAL
logging).

In pg_restore AFAICT the suggested way to handle similar FK errors is to
add another option: --disable-triggers. This works for other operations
besides TRUNCATE, and one can break referential integrity in order to
speed up loading of data.

As such, I think the proper solution would be to change TRUNCATE
implementation in the backend, in order to ignore FK constraints when
the triggers are off. How correct do you think this is? And any ideas on
how to approach the implementation?

Thanks,
Dimitris