| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Salma Elsayed <salmasayed182003(at)gmail(dot)com> |
| Cc: | Kirk Wolak <wolakk(at)gmail(dot)com>, zsolt(dot)parragi(at)percona(dot)com, Kirill Reshke <reshkekirill(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
| Subject: | Re: PoC: Simplify recovery after dropping a table by LOGGING the restore LSN |
| Date: | 2026-10-29 15:24:14 |
| Message-ID: | f3a7f10e-49e0-4a7b-bde3-23acdc2ce596@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Salma
On 28/10/2026 21:17, Salma Elsayed wrote:
> Attached is v11, which removes DROP DATABASE as discussed, applies the
> error style guide to the log messages, and simplifies the TAP test.
Thanks for updating the patch.
A few questions:
== callback functions ==
The comments of RegisterXactCallback and RegisterSubXactCallback suggest
they're meant for extensions: "These functions are intended for use by
dynamically loaded modules.". Are they really the best fit here?
== RegisterDropOrTruncateTable ==
The function already expects reloid, so it could look for rel_name and
rel_namespace itself if needed -- currently the caller needs to do it.
Have you considered simply (relid, is_truncate)?
== doDeletion ==
Is the relname != NULL guard and the "unknown" schemaname really
necessary? I imagined that at this point the relation is locked and must
exist. Also, IMHO silently skipping a log entry or just logging a
placeholder is arguably not a good idea for this feature.
== prepared transactions ==
In DropTableXactCallback there is a whole else if block just to log that
logging the lsn is not possible. Also, I'd argue that the log message in
the following scenario is not very helpful:
psql (20devel)
Type "help" for help.
postgres=# create table t as select 1;
SELECT 1
postgres=# set client_min_messages to log;
SET
postgres=# set log_object_drops to on;
SET
postgres=# begin;
BEGIN
postgres=*# drop table t;
DROP TABLE
postgres=*# prepare transaction 'p';
LOG: table "public.t" (OID 27558) is being dropped inside a prepared
transaction
DETAIL: Automatic recovery LSN capture is not supported for two-phase
commit.
HINT: If this table needs to be recovered, the WAL around the eventual
COMMIT PREPARED will need to be inspected manually.
PREPARE TRANSACTION
postgres=# rollback prepared 'p';
ROLLBACK PREPARED
postgres=# select * from t;
?column?
----------
1
(1 row)
A simplistic approach would be just to make it explicit in the docs and
not log anything.
Thanks for working on this!
Best, Jim
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Burd | 2026-10-29 15:27:14 | Re: Quickselect instead of a heap for ORDER BY ... LIMIT N large bounded sorts |
| Previous Message | Manu | 2026-10-29 15:19:24 | Re: Count the last block of the validation scan in CIC progress |