| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(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-05 05:09:47 |
| Message-ID: | CAFiTN-vWNHh0e2gXHdWr_efg0bMTcKJP1eWvvADMTf9p4a2MyA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Dec 4, 2025 at 8:05 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 3 Dec 2025 at 16:57, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Wed, Dec 3, 2025 at 9:49 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > >
> > > > relid | 16391
> > > > schemaname | public
> > > > relname | conf_tab
> > > > conflict_type | multiple_unique_conflicts
> > > > remote_xid | 761
> > > > remote_commit_lsn | 0/01761400
> > > > remote_commit_ts | 2025-12-02 15:02:07.045935+00
> > > > remote_origin | pg_16406
> > > > key_tuple |
> > > > remote_tuple | {"a":2,"b":3,"c":4}
> > > > local_conflicts |
> > > > {"{\"xid\":\"773\",\"commit_ts\":\"2025-12-02T15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":2,\"b\":2,\"c\":2}}","{\"xid\":\"
> > > > 773\",\"commit_ts\":\"2025-12-02T15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":3,\"b\":3,\"c\":3}}","{\"xid\":\"773\",\"commit_ts\":\"2025-12-02T
> > > > 15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":4,\"b\":4,\"c\":4}}"}
> > > >
> > >
> > > Thanks, it looks good. For the benefit of others, could you include a
> > > brief note, perhaps in the commit message for now, describing how to
> > > access or read this array column? We can remove it later.
> >
> > Thanks, okay, temporarily I have added in a commit message how we can
> > fetch the data from the JSON array field. In next version I will add
> > a test to get the conflict stored in conflict log history table and
> > fetch from it.
>
> I noticed that the table structure can get changed by the time the
> conflict record is prepared. In ReportApplyConflict(), the code
> currently prepares the conflict log tuple before deciding whether the
> insertion will be immediate or deferred:
> + /* Insert conflict details to conflict log table. */
> + if (conflictlogrel)
> + {
> + /*
> + * Prepare the conflict log tuple. If the error level
> is below ERROR,
> + * insert it immediately. Otherwise, defer the
> insertion to a new
> + * transaction after the current one aborts, ensuring
> the insertion of
> + * the log tuple is not rolled back.
> + */
> + prepare_conflict_log_tuple(estate,
> +
> relinfo->ri_RelationDesc,
> +
> conflictlogrel,
> + type,
> + searchslot,
> +
> conflicttuples,
> + remoteslot);
> + if (elevel < ERROR)
> + InsertConflictLogTuple(conflictlogrel);
> +
> + table_close(conflictlogrel, RowExclusiveLock);
> + }
>
> If the conflict history table defintion is changed just before
> prepare_conflict_log_tuple, the tuple creation will crash:
> Program received signal SIGSEGV, Segmentation fault.
> 0x00005a342e01df4f in VARATT_CAN_MAKE_SHORT (PTR=0x4000) at
> ../../../../src/include/varatt.h:419
> 419 return VARATT_IS_4B_U(PTR) &&
> (gdb) bt
> #0 0x00005a342e01df4f in VARATT_CAN_MAKE_SHORT (PTR=0x4000) at
> ../../../../src/include/varatt.h:419
> #1 0x00005a342e01e5ed in heap_compute_data_size
> (tupleDesc=0x7ab405e5dda8, values=0x7ffd7af3ad20,
> isnull=0x7ffd7af3ad15) at heaptuple.c:239
> #2 0x00005a342e0200dd in heap_form_tuple
> (tupleDescriptor=0x7ab405e5dda8, values=0x7ffd7af3ad20,
> isnull=0x7ffd7af3ad15) at heaptuple.c:1158
> #3 0x00005a342e55e8c2 in prepare_conflict_log_tuple
> (estate=0x5a3467944530, rel=0x7ab405e594e8,
> conflictlogrel=0x7ab405e5da88, conflict_type=CT_INSERT_EXISTS,
> searchslot=0x0,
> conflicttuples=0x5a3467942da0, remoteslot=0x5a346792e498) at conflict.c:936
> #4 0x00005a342e55cea6 in ReportApplyConflict (estate=0x5a3467944530,
> relinfo=0x5a346792e778, elevel=21, type=CT_INSERT_EXISTS,
> searchslot=0x0, remoteslot=0x5a346792e498,
> conflicttuples=0x5a3467942da0) at conflict.c:168
> #5 0x00005a342e348c35 in CheckAndReportConflict
> (resultRelInfo=0x5a346792e778, estate=0x5a3467944530,
> type=CT_INSERT_EXISTS, recheckIndexes=0x5a3467942648, searchslot=0x0,
> remoteslot=0x5a346792e498) at execReplication.c:793
>
> This can be reproduced by the following steps:
> CREATE PUBLICATION pub;
> CREATE SUBSCRIPTION sub ... WITH (conflict_log_table = 'conflict');
> ALTER TABLE conflict RENAME TO conflict1:
> CREATE TABLE conflict(c1 varchar, c2 varchar);
> -- Cause a conflict, this will crash while trying to prepare the
> conflicting tuple
Yeah while it is allowed to drop or alter the conflict log table, it
should not seg fault, IMHO error is acceptable as per the initial
discussion, so I will look into this and tighten up the logic so that
it will throw an error whenever it can not insert into the conflict
log table.
--
Regards,
Dilip Kumar
Google
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dilip Kumar | 2025-12-05 05:16:44 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Donghang Lin | 2025-12-05 04:43:12 | Re: bt_index_parent_check and concurrently build indexes |