| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Antonin Houska <ah(at)cybertec(dot)at> |
| Cc: | shihao zhong <zhong950419(at)gmail(dot)com>, Manu <manuelreyesbravo(at)gmail(dot)com>, Thom Brown <thom(at)linux(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: REPACK (CONCURRENTLY) can silently lose updates when the toast table is rewritten |
| Date: | 2026-09-23 18:27:15 |
| Message-ID: | CAD21AoCYvsp5HFOXLx9BpSz6Lcyw7H9-RXtq-b=AA8joP8=-XA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Sep 23, 2026 at 9:23 AM Antonin Houska <ah(at)cybertec(dot)at> wrote:
>
> shihao zhong <zhong950419(at)gmail(dot)com> wrote:
>
> > > or whether the relfilenode should be re-checked after the snapshot is built
> >
> > Holding the toast lock from the start deadlocks. A session that asks for
> > AccessExclusiveLock gets an XID before it waits, and the decoding worker
> > waits for all XIDs while it sets up.
>
> The same (supposedly low) deadlock risk already exists for the main table, see
> this comment in rebuild_relation():
>
> /*
> * Start the worker that decodes data changes applied while we're
> * copying the table contents.
> *
> * Note that the worker has to wait for all transactions with XID
> * already assigned to finish. If some of those transactions is
> * waiting for a lock conflicting with ShareUpdateExclusiveLock on our
> * table (e.g. it runs CREATE INDEX), we can end up in a deadlock.
> * Not sure this risk is worth unlocking/locking the table (and its
> * clustering index) and checking again if it's still eligible for
> * REPACK CONCURRENTLY.
> */
> start_repack_decoding_worker(tableOid);
>
> I'm not sure if locking the TOAST relation earlier would make the situation
> worse.
Agreed.
So I think the simplest fix would be to acquire a lock on the TOAST
table before starting the repack worker. It would make the case in
question fail with a deadlock, instead of silently losing updates.
The proposed patch also fixes the problem, but I'm concerned that it
repeatedly starts and stops the repack worker without any limit. I
think we could error out if we detect a concurrent rewrite, so that
users can re-run REPACK CONCURRENTLY. This check could also be done on
the repack worker side: after getting the relfilelocator of the TOAST
table and initializing the logical decoding, the repack worker
rechecks the relfilelocator. If they don't match, it raises an error.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-09-23 18:29:45 | Re: Add a test for index_rebuild_count of REPACK (CONCURRENTLY) |
| Previous Message | Corey Huinker | 2026-09-23 18:26:28 | Re: Import Statistics in postgres_fdw before resorting to sampling. |