| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | Thom Brown <thom(at)linux(dot)com> |
| Cc: | Antonin Houska <ah(at)cybertec(dot)at>, shihao zhong <zhong950419(at)gmail(dot)com>, Robert Treat <rob(at)xzilla(dot)net>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Manu <manuelreyesbravo(at)gmail(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-25 14:56:58 |
| Message-ID: | araJcIXmBySfiXzc@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-Sep-25, Thom Brown wrote:
> Moving it into cluster_rel() makes sense, but does it need to be the
> full ShareUpdateExclusiveLock there?
Yeah, it's better to acquire the lock you want upfront, because
otherwise you introduce more risk of deadlock caused by lock upgrades
(admittedly the user would have to be doing something really stupid in
order for this to be a real problem, but still.) It's only AEL that we
don't want to hold for long.
Maybe the patch could be somewhat like this, then? I didn't review the
test carefully other than running without the code fix to verify that it
fails, and then passes with the fix; and I didn't read the commit
messages either, which I think are LLM-written and not really correct.
(Also, I would push both things as a single commit.)
I think changing the lock as obtained by copy_table_data is not very
nice, because that one is unconditional, and here we only want it in
concurrent mode. BTW I noticed that the comment for copy_table_data
mentions decoding_ctx as an argument, which doesn't exist.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-Test-TOAST-rewrite-during-REPACK-CONCURRENTLY-sta.patch | text/x-diff | 7.2 KB |
| v4-0002-Fix-REPACK-CONCURRENTLY-losing-updates-after-a-TO.patch | text/x-diff | 3.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ayush Tiwari | 2026-09-25 15:24:58 | Re: Add a pg_wal_preallocate() SQL function to eagerly create future WAL segments |
| Previous Message | Andrei Lepikhov | 2026-09-25 14:53:39 | Redesign the EXPLAIN metric ‘Removed by Join Filter’? |