Re: Proposal: Conflict log history table for Logical Replication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: shveta malik <shveta(dot)malik(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: 2025-12-15 09:48:23
Message-ID: CAA4eK1K8Aqm+jP_EMPF8H_3UJSEExdwDCaphq6=unZZMdcmD0A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Dec 14, 2025 at 9:16 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> Here is the patch which implements the dependency and fixes other
> comments from Shveta.
>

+/*
+ * Check if the specified relation is used as a conflict log table by any
+ * subscription.
+ */
+bool
+IsConflictLogTable(Oid relid)
+{
+ Relation rel;
+ TableScanDesc scan;
+ HeapTuple tup;
+ bool is_clt = false;
+
+ rel = table_open(SubscriptionRelationId, AccessShareLock);
+ scan = table_beginscan_catalog(rel, 0, NULL);
+
+ while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection)))

This function has been used at multiple places in the patch, though
not in any performance-critical paths, but still, it seems like the
impact can be noticeable for a large number of subscriptions. Also, I
am not sure it is a good design to scan the entire system table to
find whether some other relation is publishable or not. I see below
kinds of usages for it:

+ /* Subscription conflict log tables are not published */
+ result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple)) &&
+ !IsConflictLogTable(relid);

In this regard, I see a comment atop is_publishable_class which
suggests as follows:

The best
* long-term solution may be to add a "relispublishable" bool to pg_class,
* and depend on that instead of OID checks.
*/
static bool
is_publishable_class(Oid relid, Form_pg_class reltuple)

I feel that is a good idea for reasons mentioned atop
is_publishable_class and for the conflict table. What do you think?

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Arunprasad Rajkumar 2025-12-15 09:48:56 Re: [PATCH] Skip unpublishable child tables when adding parent to publication
Previous Message Soumya S Murali 2025-12-15 09:37:00 Re: Checkpointer write combining