| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-06-13 11:27:54 |
| Message-ID: | CAA4eK1K1vgau8UQZT88kj9dFMe8gfVxyFe4zpaGKaKxQqCh+Dw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Jun 13, 2026 at 3:46 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Jun 11, 2026 at 5:53 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Thu, 11 Jun 2026 at 10:44, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > Please find the rebased patch
> > > 1. It includes the new 0005 patch for reporting errors for DDLs on clt.
> > >
> > > Open comments:
> > > 1. Recent comments from Nisha and Shveta after v47 are still open
> > > 2. Vignesh's patch for "describe related" changes needs a rebase. Can
> > > you do that, Vignesh? Meanwhile, I will close all the open comments
> > > and try to share a new version by EOD today.
> >
> > Here is the rebased version of the patch attached.
>
> Please find attached the latest patch. I have reordered the series,
> moving 0006 to 0002, and updated the lock restrictions. We now allow
> ACCESS SHARE mode exclusively to ensure pg_dump can acquire its
> necessary locks, while blocking higher-level locks to prevent
> interference with insertions into the conflict log tables.
>
+ /*
+ * Conflict log tables are managed by the system for logical replication
+ * conflicts and should not be locked explicitly. However, AccessShareLock
+ * is allowed to support pg_dump, which must lock tables to prevent them
+ * from being dropped or altered between fetching the table list and
+ * performing the dump. This read-only lock is safe because it does not
+ * interfere with concurrent insertions into the conflict log tables.
+ */
+ if (IsConflictLogTableNamespace(get_rel_namespace(relid)) &&
+ lockmode > AccessShareLock)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a conflict log table",
+ rv->relname),
+ errdetail("Conflict log tables are system-managed and cannot be
locked explicitly, except in ACCESS SHARE mode.")));
In favor of keeping this code simple, can we allow locking the CLT
table with the following comment: "Note: Conflict log tables are
deliberately NOT blocked here, even though other direct DDL on them is
rejected elsewhere. pg_dump relies on being able to take an ACCESS
SHARE lock on these tables to safely dump their definitions during
binary upgrade, so we permit LOCK so we allow LOCK on them and treat
them like ordinary tables here.
It's true that a strong lock (ShareLock or above) on such a table
would conflict with the RowExclusiveLock taken by the apply worker's
insertsand could stall conflict logging for as long as it is held.
But locking a system-managed conflict log table is an unusual thing to
do, and it doesn't seem worth the trouble of filtering by lock mode
here."
--
With Regards,
Amit Kapila.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-06-13 12:14:02 | Re: Change copyObject() to use typeof_unqual |
| Previous Message | Dilip Kumar | 2026-06-13 10:16:40 | Re: Proposal: Conflict log history table for Logical Replication |