Re: Proposal: Conflict log history table for Logical Replication

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, shveta malik <shveta(dot)malik(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-09 15:11:21
Message-ID: CAFiTN-sJbhPX+LbA8YuQeYJpfGA2XA+OKXf8jCm04RoJOyzLvw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 9, 2025 at 12:06 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Dec 9, 2025 at 10:12 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Dec 8, 2025 at 5:15 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Dec 8, 2025 at 3:21 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Mon, Dec 8, 2025 at 3:01 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > > >
> > > > > On Mon, Dec 8, 2025 at 2:38 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > > >
> > > > > > On Mon, Dec 8, 2025 at 10:25 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > > > > >
> > > > > > > On Thu, Dec 4, 2025 at 4:20 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Dec 4, 2025 at 10:49 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > > I think the conflict history table should not be transferred to the
> > > > > > > > > > new cluster when pg_upgrade since the table definition could be
> > > > > > > > > > different across major versions.
> > > > > > > > >
> > > > > > > > > Let me think more on this with respect to behaviour of other factors
> > > > > > > > > like subscriptions etc.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Can we deal with different schema of tables across versions via
> > > > > > > > pg_dump/restore during upgrade?
> > > > > > > >
> > > > > > >
> > > > > > > While handling the case of conflict_log_table option during pg_dump, I
> > > > > > > realized that the restore is trying to create conflict log table 2
> > > > > > > different places 1) As part of the regular table dump 2) As part of
> > > > > > > the CREATE SUBSCRIPTION when conflict_log_table option is set.
> > > > > > >
> > > > > > > So one option is we can avoid dumping the conflict log tables as part
> > > > > > > of the regular table dump if we think that we do not need to conflict
> > > > > > > log table data and let it get created as part of the create
> > > > > > > subscription command, OTOH if we think we want to keep the conflict
> > > > > > > log table data,
> > > > > > >
> > > > > >
> > > > > > We want to retain conflict_history after upgrade. This is required for
> > > > > > various reasons (a) after upgrade DBA user will still require to
> > > > > > resolved the pending unresolved conflicts, (b) Regulations often
> > > > > > require keeping audit trails for a longer period of time. If a
> > > > > > conflict occurred at time X (which is less than the regulations
> > > > > > requirement) regarding a financial transaction, that record must
> > > > > > survive the upgrade, (c)
> > > > > > If something breaks after the upgrade (e.g., missing rows, constraint
> > > > > > violations), conflict history helps trace root causes. It shows
> > > > > > whether issues existed before the upgrade or were introduced during
> > > > > > migration, (d) as users can query the conflict_history tables, it
> > > > > > should be treated similar to user tables.
> > > > > >
> > > > > > BTW, we are also planning to migrate commit_ts data in thread [1]
> > > > > > which would be helpful for conflict_resolutions after upgrade.
> > > > > >
> > > > > > let it get dumped as part of the regular tables and in
> > > > > > > CREATE SUBSCRIPTION we will just set the option but do not create the
> > > > > > > table,
> > > > > > >
> > > > > >
> > > > > > Yeah, we can turn this option during CREATE SUBSCRIPTION so that it
> > > > > > doesn't try to create the table again.
> > > > > >
> > > > > > > although we might need to do special handling of this case
> > > > > > > because if we allow the existing tables to be set as conflict log
> > > > > > > tables then it may allow other user tables to be set, so need to think
> > > > > > > how to handle this if we need to go with this option.
> > > > > > >
> > > > > >
> > > > > > Yeah, probably but it should be allowed internally only not to users.
> > > > >
> > > > > Yeah I wanted to do that, but problem is with dump and restore, I mean
> > > > > if you just dump into a sql file and execute the sql file at that time
> > > > > the CREATE SUBSCRIPTION with conflict_log_table option will fail as
> > > > > the table already exists because it was restored as part of the dump.
> > > > > I know under binary upgrade we have binary_upgrade flag so can do
> > > > > special handling not sure how to distinguish the sql executing as part
> > > > > of the restore or normal sql execution by user?
> > > > >
> > > >
> > > > See dumpSubscription(). We always use (connect = false) while dumping
> > > > subscription, so, similarly, we should always dump the new option with
> > > > default value which not to create the history table. Won't that be
> > > > sufficient?
> > >
> > > Thinking out loud, so basically what we need is we need to create
> > > subscription and set the conflict log table in catalog entry of the
> > > subscription in pg_subscription but do not want to create the conflict
> > > log table, so seems like we need to invent something new which set the
> > > conflict log table in catalog but do not create the table. Currently
> > > we have a single option that if conflict_log_table='table_name' is set
> > > then we will create the table as well as set the table name in the
> > > catalog, so need to think of something on the line of separating this,
> > > or something more innovative.
> > >
> >
> > This needs more thought and discussion, so it is better to separate
> > out this part at this stage and let's try to review the core patch
> > first.
>
> +1
>
> BTW, I told a few days back to have two options (instead of a
> > single option conflict_log_table) to allow extension of more ways to
> > LOG the conflict data.
>
> Yeah, I will put that as well in an add on patch, once I fix all the
> option issues of the core patch.
>
Here is the updated version of patch
What has changed
1. Table is created using create_heap_with_catalog() instead of SPI as
suggested by Sawada-San and Amit Kapila.
2. Validated the table schema after acquiring the lock before
preparing/inserting conflict tuples for defects raised by Vignesh.
3. Bug fixes raised by Shweta (segfault)
3. Comments from Peter (except exposing namespace in \dRs+, it's still pending.

What's not done/pending
1. Adding for key_tuple/RI as pointed by Shveta - will do in next version
2. Adding dependency of subscription on table so that we are not
allowed to drop the table - I think when we put the dependency on
shared objects those can not be dropped even with cascade option, but
I am still exploring more on this.
3. dump/restore and upgrade, I have partially working patch but then I
need to figure out how to skip table creation while creating
subscription, while discussing offlist with Hannu, he suggested we can
do something with dump dependency ordering, e.g. we can dump create
subscription first and then dump the clt data without actually dumping
the clt definition, with that table will be created while creating the
subscription and then data will be restored with COPY command, I will
explore more on this.
4. Test case for conflit insertion
5. Documentation patch

--
Regards,
Dilip Kumar
Google

Attachment Content-Type Size
v10-0001-Add-configurable-conflict-log-table-for-Logical-.patch application/octet-stream 122.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2025-12-09 15:14:44 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Previous Message Jelte Fennema-Nio 2025-12-09 14:45:43 Re: Type assertions without GCC builtins