| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | 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-11-13 15:47:11 |
| Message-ID: | CAFiTN-vFV9-zajrwjYHYyFnyQsooOAXW4CpxB5f-iT3APjOtoQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Nov 13, 2025 at 2:39 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> > Few observations related to publication.
> > ------------------------------
Thanks Shveta, for testing and sharing your thoughts. IMHO for
conflict log tables it should be good enough if we restrict it when
ALL TABLE options are used, I don't think we need to put extra effort
to completely restrict it even if users want to explicitly list it
into the publication.
> >
> > (In the below comments, clt/CLT implies Conflict Log Table)
> >
> > 1)
> > 'select pg_relation_is_publishable(clt)' returns true for conflict-log table.
This function is used while publishing every single change and I don't
think we want to add a cost to check each subscription to identify
whether the table is listed as CLT.
> > 2)
> > '\d+ clt' shows all-tables publication name. I feel we should not
> > show that for clt.
I think we should fix this.
> > 3)
> > I am able to create a publication for clt table, should it be allowed?
I believe we should not do any specific handling to restrict this but
I am open for the opinions.
> > create subscription sub1 connection '...' publication pub1
> > WITH(conflict_log_table='clt');
> > create publication pub3 for table clt;
> >
> > 4)
> > Is there a reason we have not made '!IsConflictHistoryRelid' check as
> > part of is_publishable_class() itself? If we do so, other code-logics
> > will also get clt as non-publishable always (and will solve a few of
> > the above issues I think). IIUC, there is no place where we want to
> > mark CLT as publishable or is there any?
IMHO the main reason is performance.
> > 5) Also, I feel we can add some documentation now to help others to
> > understand/review the patch better without going through the long
> > thread.
Make sense, I will do that in the next version.
> >
> > Few observations related to conflict-logging:
> > ------------------------------
> > 1)
> > I found that for the conflicts which ultimately result in Error, we do
> > not insert any conflict-record in clt.
> >
> > a)
> > Example: insert_exists, update_Exists
> > create table tab1 (i int primary key, j int);
> > sub: insert into tab1 values(30,10);
> > pub: insert into tab1 values(30,10);
> > ERROR: conflict detected on relation "public.tab1": conflict=insert_exists
> > No record in clt.
> >
> > sub:
> > <some pre-data needed>
> > update tab1 set i=40 where i = 30;
> > pub: update tab1 set i=40 where i = 20;
> > ERROR: conflict detected on relation "public.tab1": conflict=update_exists
> > No record in clt.
Yeah that interesting need to put thought on how to commit this record
when an outer transaction is aborted as we do not have autonomous
transactions which are generally used for this kind of logging. But
we can explore more options like inserting into conflict log tables
outside the outer transaction.
> > b)
> > Another question related to this is, since these conflicts (which
> > results in error) keep on happening until user resolves these or skips
> > these or 'disable_on_error' is set. Then are we going to insert these
> > multiple times? We do count these in 'confl_insert_exists' and
> > 'confl_update_exists' everytime, so it makes sense to log those each
> > time in clt as well. Thoughts?
I think it make sense to insert every time we see the conflict, but it
would be good to have opinion from others as well.
> > 2)
> > Conflicts where row on sub is missing, local_ts incorrectly inserted.
> > It is '2000-01-01 05:30:00+05:30'. Should it be Null or something
> > indicating that it is not applicable for this conflict-type?
> >
> > Example: delete_missing, update_missing
> > pub:
> > insert into tab1 values(10,10);
> > insert into tab1 values(20,10);
> > sub: delete from tab1 where i=10;
> > pub: delete from tab1 where i=10;
Sure I will test this.
>
> 3)
> We also need to think how we are going to display the info in case of
> multiple_unique_conflicts as there could be multiple local and remote
> tuples conflicting for one single operation. Example:
>
> create table conf_tab (a int primary key, b int unique, c int unique);
>
> sub: insert into conf_tab values (2,2,2), (3,3,3), (4,4,4);
>
> pub: insert into conf_tab values (2,3,4);
>
> ERROR: conflict detected on relation "public.conf_tab":
> conflict=multiple_unique_conflicts
> DETAIL: Key already exists in unique index "conf_tab_pkey", modified
> locally in transaction 874 at 2025-11-12 14:35:13.452143+05:30.
> Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
> Key already exists in unique index "conf_tab_b_key", modified locally
> in transaction 874 at 2025-11-12 14:35:13.452143+05:30.
> Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
> Key already exists in unique index "conf_tab_c_key", modified locally
> in transaction 874 at 2025-11-12 14:35:13.452143+05:30.
> Key (c)=(4); existing local row (4, 4, 4); remote row (2, 3, 4).
> CONTEXT: processing remote data for replication origin "pg_16392"
> during message type "INSERT" for replication target relation
> "public.conf_tab" in transaction 781, finished at 0/017FDDA0
>
> Currently in clt, we have singular terms such as 'key_tuple',
> 'local_tuple', 'remote_tuple'. Shall we have multiple rows inserted?
> But it does not look reasonable to have multiple rows inserted for a
> single conflict raised. I will think more about this.
Currently I am inserting multiple records in the conflict history
table, the same as each tuple is logged, but couldn't find any better
way for this. Another option is to use an array of tuples instead of a
single tuple but not sure this might make things more complicated to
process by any external tool. But you are right, this needs more
discussion.
--
Regards,
Dilip Kumar
Google
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2025-11-13 15:53:14 | Re: pg_getaddrinfo_all() with hintp=NULL |
| Previous Message | Tom Lane | 2025-11-13 15:26:32 | Re: pg_getaddrinfo_all() with hintp=NULL |