public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: shveta malik <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: Mon, 22 Jun 2026 14:37:53 +0530
Message-ID: <CAA4eK1J+PRbKpPWLNBc6UJBgsUFbnczkwsKAQegEiXmLP=ng0w@mail.gmail.com> (raw)
In-Reply-To: <CAJpy0uADR=MkBZAswgNTow6DZQ+AQ-PPcSKCgbCGfF=HYdgxWw@mail.gmail.com>
References: <CAFiTN-u5D5o_AGNbHRZHaOqAMWkxLf+hSk_r9X3gv6HbLOB5+g@mail.gmail.com>
<CALj2ACViThGQDYi-yeqUeHqG2Pozn2AiyvtDtjE6zhhbM0KsEA@mail.gmail.com>
<CAA4eK1+44b3vd_OWfiaVNtjf5Njb5cek09pmKRmttBByeg0NoA@mail.gmail.com>
<CAFiTN-v3L0WacCDx5dkOSonaZQbJfstXL4HrCPD1ahRdUsRnSg@mail.gmail.com>
<CALj2ACW63uuxh0fSoxEAF8OMWhz1dJKSkp268WJDzf5BUqCf5g@mail.gmail.com>
<CAFiTN-s9WWLOhW1TO27NtJwGf0bh2+MWyp3NEkZFeN_S5_p_rA@mail.gmail.com>
<CAA4eK1LxnsEx5sMbQkK5MHAgXKPROMQXQ0n=fKMwz+UsfKQaMQ@mail.gmail.com>
<CAD21AoDj+c4LXf2y4ESR-gVyv9d8V0G4R8R9pn-PcmT5zPzYcg@mail.gmail.com>
<CAA4eK1KokmAwNOL6bS-ip_E3F96PiQTjC4j-M+5vD1T6uUyi3Q@mail.gmail.com>
<CAFiTN-vFKE8E_N6h+peX9DP92mxCeFdm5A9Esn4DkLmNcZ-dOA@mail.gmail.com>
<CAFiTN-shLYf-fOTQ_dBf3Xfx05gxs_8d93MHZXyyz6w2Bg5geQ@mail.gmail.com>
<CAFiTN-tEgkKQHUikn6iBFCYf7XOObR7ncUq=OVh7WEk=6P4ymw@mail.gmail.com>
<CAFiTN-tQiakd8m+-d6WN6RpJXSv_JcropZ2oGzme4d1JudQhYg@mail.gmail.com>
<CAJpy0uDKbYWt+YPADj=4fHEvrGEWgnG1n_YsiGT_EZiZf0VSAw@mail.gmail.com>
<CAFiTN-t82BiXen+HfdR9jZyOpuSO92xonnUK=khXsiZWBfOxMA@mail.gmail.com>
<CAJpy0uAu2paxGAEffD=vaBTW9Jqbtxxawb8K8FgiASfeKPnGog@mail.gmail.com>
<CAJpy0uC0ZWgHOivJ102A1fMkppwK3RuSMafRPKyjwkmJrjhVUw@mail.gmail.com>
<CAFiTN-vFV9-zajrwjYHYyFnyQsooOAXW4CpxB5f-iT3APjOtoQ@mail.gmail.com>
<CAJpy0uBeU1dZgaqsSVKc=P=EVUKxRgVuHR8jDXFL-HLibbE-kQ@mail.gmail.com>
<CAA4eK1+FOkOxhzVLAnDymoNjp4i98H-L1+ZsWDgJEv-ndnTzTA@mail.gmail.com>
<CAFiTN-sVK6Bp+BawCJU_WpAXQSTX4OkKmce5EE4YNBgD-XSjZw@mail.gmail.com>
<CAA4eK1LbjV0bctib9wUnBpEkC+2rZFPnGuRtrKuc5AtUAzum+A@mail.gmail.com>
<CAFiTN-vq50N3QP9p3_SH+tJ8Pn=uRDb0X4qEcQZYcGW9AX88rQ@mail.gmail.com>
<CAFiTN-u3+zRGPESP5kUUfa6NxaWh1HL-gd1225KJ0Uvzi1urow@mail.gmail.com>
<CAA4eK1L4iNk6mNTC83PbYrRfUdtivH4U961PkdFfOO7mvc=USg@mail.gmail.com>
<CAFiTN-v+Mh64UfR5zb5rwgyGm6HS80XRSZ_XeaWkg8=+s9o3Kg@mail.gmail.com>
<CAFiTN-s3ZFHteQsiC3H4=AjTWxuwN-w69XQ3xL5X6YOMTua4pA@mail.gmail.com>
<CAJpy0uDe724nY59j-8hMapZ_Fru1Wo-NucF4Ea1B3Jrw=+J+UQ@mail.gmail.com>
<CAFiTN-uR=86L_5tyiA7n73EXCSCuDfQKfL5O=c8n7zZom8_ONQ@mail.gmail.com>
<CAD21AoDfOS-J0M9WbM3D20eGbSPzbfLQ-9XoYkxO4AZ9twqyvg@mail.gmail.com>
<CAFiTN-vMTg2X7vwfHLr5Gvy8ViV63_iaEcpHmM8V5GpA9-u8cg@mail.gmail.com>
<CAA4eK1+b2Ws0e_ZYJsgZAPn7VWndxAK_YM_QMKcfXst3e7F6Jg@mail.gmail.com>
<CAFiTN-v6hFKMPrSyTBsz=AtEETYMbOxrqvhZJsPQqKgQc4WCLw@mail.gmail.com>
<CAA4eK1KV3rYkaxys5fh-PtE9kq5xrFbiaRpOSPoRgQG494ek+g@mail.gmail.com>
<CAFiTN-utvu=QjY1QQ1a_TvkpkpvesMWo9M8wTFYLaOTPdpOJvw@mail.gmail.com>
<CAA4eK1+HoSOEqNwT3twArPNx4_D7hSUoEg2LnYhX8n9iUwhXgQ@mail.gmail.com>
<CAFiTN-tqmsfW0Sk=1RhzuduxqLrf9KEc8VOvBae+4aYxWTJwuA@mail.gmail.com>
<CAA4eK1JmCQ=DHe3HsqpX+P3mGDUd_Z7E7oAxdstK6822W6tuCw@mail.gmail.com>
<CAFiTN-uE4eAUYewuq3c5deAt3TtVork+H6rkUHRv68cOGr5rmQ@mail.gmail.com>
<CAFiTN-sJbhPX+LbA8YuQeYJpfGA2XA+OKXf8jCm04RoJOyzLvw@mail.gmail.com>
<CAJpy0uBPOyWj9itFjHzGXfrUuYS8KGmAvgdcV_9FPjWZ0EZz_w@mail.gmail.com>
<CAFiTN-s=iLE4qM4qmw9yXKqW09R_c_HqaSGeZXJ2EaTVfXss+g@mail.gmail.com>
<CAA4eK1KYo0vZpPSRc_4gVpa06-J39gxjs3tHFyckgkBfYJSfFA@mail.gmail.com>
<CAFiTN-vrKc6OWzrg6yvpwYcj79k=zkrDp3uwiZzjwrWLJAq6tw@mail.gmail.com>
<CAA4eK1LmvrfEgn1NUZZ=E3yMCjQdNZ5=_SBEry73-EmF6jM_PQ@mail.gmail.com>
<CAFiTN-vjfub5b3PqPQzfOw9BSjm8jt28ott+Hoz9CrRxJHzYkg@mail.gmail.com>
<CAFiTN-v=ANapYvRK+SOy2wJb4CSuD6Vb6_bTGuReM9Dv+3tucA@mail.gmail.com>
<CALDaNm1zEYoSdf2Ns-=UJRw95E5sbfpB0oaNUWtRJN27Q1Knhw@mail.gmail.com>
<CALDaNm3USsXVNBsfdpkp60HVgrTV4taWMk1xZYNBa7QUF=V0jg@mail.gmail.com>
<CAFiTN-sNg9ghLNkB2Kn0SwBGOub9acc99XZZU_d5NAcyW-yrEg@mail.gmail.com>
<CAJpy0uAF3EYcYdpTHdKMeXfvaPbNvnWrZUATrSLL1hqjao=33A@mail.gmail.com>
<CAFiTN-uikggCKp2LscTorKY5d3KF9j93DW0xebDcRX86G+ZsSw@mail.gmail.com>
<CAJpy0uDaOoVK8S3_xxTAcTDpfK1AY7tApw7nPOZG_gUz+DMi=Q@mail.gmail.com>
<CAA4eK1+AdeC5B9xrAXSKWGtTh-0d8xdD=fZttmOBm+c8o8thAQ@mail.gmail.com>
<CAFiTN-skBQAeuzuUd+PDK0Gqc8g+4x9ypBMwJhOrmW8ZCFKGSA@mail.gmail.com>
<CAJpy0uCdrsW5T+okq7xTOVxagje7FW3DOeY5B0CGKYa5VqF_tQ@mail.gmail.com>
<CAFiTN-u+_mFj9caYYFO7=_YHFXk5y=vvOm2H2=5hctYktmAVGA@mail.gmail.com>
<CALDaNm1aivk9KgQ5daeF6YZzuE+0wWc2yb7wb6qikNyvfPN0Sg@mail.gmail.com>
<CAJpy0uD6fTEUYJx3+yDbvB=VW7c5AaGoeSd7iwHdYYO=kYGn3g@mail.gmail.com>
<CALDaNm2YOOdJ25X1sJ+DYz37K6Qi4g0ZNFHb_pQMF9UqancnEA@mail.gmail.com>
<CAHut+PtMS5bENS0DVtBj+s3kUEOq61+hSkqLODjFB78egB0imQ@mail.gmail.com>
<CAFiTN-s_M83sfs+MHHbUrMesjsCPN4JWxY5MChCEiY1U-u7=9g@mail.gmail.com>
<CAFiTN-vj8NTm9w_L2XdhxJCub_RZw__YVUgfXa1B1kJzJctRNw@mail.gmail.com>
<CAJpy0uBDLnfhuSiev8W9ZMFNTzUmqhds2dKayUpLoN-z1dtsLA@mail.gmail.com>
<CAFiTN-uL9f0X+=Ep4BbAPvaTJA7S4XHM--G4BsnPJw4uJW7EGQ@mail.gmail.com>
<CAJpy0uDG=t-y_m8t1zpBzfz9viP3K8dyQgkruaraVT85UtTkrg@mail.gmail.com>
<CAFiTN-tR8Rhs8uhfbck0Ac4dd1MopvvYgjK39nWyNXRp9Z3Qww@mail.gmail.com>
<CAA4eK1Kf15UpNmpTTE2XyX=9PE_oTpOoy5xqg3rFWbxwwP4Rbg@mail.gmail.com>
<CAFiTN-tNqb0vjuadDz-as67ksSXa=aEK+JW=4b54RVmkUK1m2Q@mail.gmail.com>
<CAFiTN-vDCxx6ydUFo59L8qNBbierg4as3TGPPiavR7UZjYurzA@mail.gmail.com>
<CAHut+PsWms218ENALnytLEV4NpxjOrAYhChLDaMaeE65-vNgrQ@mail.gmail.com>
<CAFiTN-v9i9RmDvdUmtMUow4=b+nr0k7LKMyEQ+6ZF=EVdfBhBA@mail.gmail.com>
<CALDaNm2YTKwPDjt9OV49RgM0zbkWhMhNu228bj_7f+zzcPb-ew@mail.gmail.com>
<CAFiTN-t_4XvofM3an-WmykqnPE+9wf9U+o2M7p1CWd9eXkN88Q@mail.gmail.com>
<CAHut+PuaqNDfDu_3xkZR4OYxw-B7ew_WjpLXCBvMcSBJz2K6Xg@mail.gmail.com>
<CAFiTN-uqNN9S_hRuda_th5MEpywa15g+XO00yM6tNJ-spGRRJw@mail.gmail.com>
<CABdArM6QxXatkGefTHy__HgaYHBvbKesffeXzT8Vn-kvcvGK4w@mail.gmail.com>
<CAFiTN-tgMWr=TGPhs9BxaPuSC_jhM7sJJ4fHedE5W6=h40jLfA@mail.gmail.com>
<CABdArM5fgzfyC2mH3YGB8t8cJBHWqAG1BS6rJMk7mX-8=9d=Cg@mail.gmail.com>
<CABdArM568KF4WXdFX_aZkCiDK8R71Wpep0gC2a+cV8BMobwkrg@mail.gmail.com>
<CAFiTN-vQ0tu18BD3UmKPb0rzZyFMQAVgGbdpMA8iYLX7PZOqOA@mail.gmail.com>
<CALDaNm20PDtmG2E3qaTC+YuL5twv+c9k573wL3sb=OwgmZphxQ@mail.gmail.com>
<CAA4eK1LhOHa_TEznw+gFoq+w0vMvvsDG2g9Xq8Mwa8xZMY73og@mail.gmail.com>
<CAFiTN-vPDqrQ2rHykNgd+groFxqwBYFQF97R-Co2EmtUkV6MTg@mail.gmail.com>
<CAFiTN-vsd=wNiEPXPQhZnipAb--+mBUC01M-pcjBjbRockgCUA@mail.gmail.com>
<CAJpy0uCjSq_gUCJBfURhqtB6bLvkKSUL-sVXpaGKjEapv5+t+Q@mail.gmail.com>
<CAFiTN-uZ-LaStAY3NuCY-nb7GCB9joiHX7HtHEMseJ0xfnqVSg@mail.gmail.com>
<CAFiTN-vhJxRW5NQ628oidnk0KtHwKt11dW9-+vxqpXLTgjiYiA@mail.gmail.com>
<CALDaNm1cJURibYKY4+DuNosjM72C9oGheUF-roMyff__+AsKBw@mail.gmail.com>
<CAJpy0uD1_77TDAFc4jE-94X-WUus7Q3gGU0pXfC+Tticq1hFvA@mail.gmail.com>
<CAA4eK1LFcSc4XCj4mU-cv27F_6n6=+ehJ=YAsAnyBbz4Sv_tVg@mail.gmail.com>
<CAFiTN-s5ZtjXKrSbam7TNWJ9Ax-kCancXcestAnx2by7dK0-UA@mail.gmail.com>
<CAFiTN-u=Da32mXyz8jocEGtuLSG4ccXXj_aEzUTPp2zkLb3MVA@mail.gmail.com>
<CALDaNm1qY5e0thfsDB2uWXqZn4hgTWTxiUDwcF1hWA-jodsKYg@mail.gmail.com>
<CALDaNm1nFtv3dtdRdbqWo2Rf_av7XbxDfK1Orqjcqs_Su_cLRQ@mail.gmail.com>
<CABdArM7R498qC5Fr42aU_q-2Sc5QsT4dyKgmO_f6Uy=8oCAFXA@mail.gmail.com>
<CAFiTN-sRZ+Z_9B3ue2L4zkbcfmPjjcAjcR1C+px1PyAs+HGsSg@mail.gmail.com>
<CAFiTN-sdcjf9xJ2M-=ab5e4y662tTmFFiP4gHL44tC9PcQozcw@mail.gmail.com>
<CALDaNm2WNjaNxUijVkvT6y69D62rfCu8OMwU-Pf-84un2r_=ig@mail.gmail.com>
<CAHut+PvEP5uUR13xJ3gbNKGU49=Rg32DXMGZ2wL9jTcKHyN_=Q@mail.gmail.com>
<CALDaNm3Jb5AQTsFJFxYZZJCaheT7qToCZkEALfW-vsMMFxjOyQ@mail.gmail.com>
<CAHut+PtQn5U9i00qvBmjo0KBxyb+ZmBb38NzF91KnX4J86Jg_g@mail.gmail.com>
<CALDaNm1a1gzy0L38U394_4OFwGUS8ALgSONYj++VLimY0g9piQ@mail.gmail.com>
<CABdArM5X63AdtS99QKGjVijUd_Q_dV8QUDSo4nTHKJjn3JwtAg@mail.gmail.com>
<CAA4eK1+h=QV4Zi=PW8Zt2D6be5ki5Mu2HgdXcfUophptx6Mt_A@mail.gmail.com>
<CAFiTN-s-tuxar9Dp5He0CFa1pzfy1fmiwcBj6PtwD0hDodE5ng@mail.gmail.com>
<CAFiTN-sx=k+Th=uYsrLcS6YMZbPVi9Wrggn1w2Nzf9MLEU7YRQ@mail.gmail.com>
<CAFiTN-u5pcgAhXyJgj+p7-xmShtp0i8xA000tzjCLFQp_zMXUA@mail.gmail.com>
<CAFiTN-tRpS7b3qFqckqFtHETj0jyzj-8SxC1arjfwf-hQd47PQ@mail.gmail.com>
<CABdArM5Ka_m_GWhL_zZbeDPKmL-Wezwb4A-NHnO-v-fRDuhA-Q@mail.gmail.com>
<CAFiTN-sqEMAbZ2pTt=zMa=918NV7HVeXF4bCOF+swtzKnTy5yQ@mail.gmail.com>
<CALDaNm0WX0Vqoy2UQZh-2TpWraf4OYn28kWe9aGR=vxKwLA+bw@mail.gmail.com>
<CAHut+Pv2uqfBGODj7BP_Oht3r-PQ6FQMT5pxABj2JUvqYuzRQQ@mail.gmail.com>
<CALDaNm2=BW4Lf+rv5CaHMVCMdxWiRLe_Qwu3BaH3wRJ8q2VaOA@mail.gmail.com>
<CAHut+Pv6ROpBaiYXojbudjiehtaE3imSURFHj5NJVXa=BNdtqw@mail.gmail.com>
<CALDaNm2r_5hmz22Gsio=OABvA=c_cr8avv=y19Epb0vKMo+QJQ@mail.gmail.com>
<CANhcyEUVWNjDpM-FnC=wYzU_pYj5CdSH5NEvsPOz__Sz0RWeyA@mail.gmail.com>
<CAFiTN-vCtwpTNB3-YQo5ZaMTrTGoxSKbWn3xwPp4sWh-nSM5PQ@mail.gmail.com>
<CALDaNm1b5XeycCiiB3yjShWxUbKcHUUWCE75ONs0W2V3goo8gA@mail.gmail.com>
<CAA4eK1+zn7p+4AuOFsXdVmHvgkOF6mncJ-Sa2Nh=YxAPfiTTSg@mail.gmail.com>
<CALDaNm20GcE2p8GfiTWdrp9T=1LTvxpws9djUkJBwZO_nyqqdQ@mail.gmail.com>
<CAJpy0uADR=MkBZAswgNTow6DZQ+AQ-PPcSKCgbCGfF=HYdgxWw@mail.gmail.com>
On Mon, Jun 22, 2026 at 1:56 PM shveta malik <[email protected]> wrote:
>
> On Mon, Jun 22, 2026 at 9:33 AM vignesh C <[email protected]> wrote:
> >
> > On Mon, 22 Jun 2026 at 08:41, Amit Kapila <[email protected]> wrote:
> > >
> > > On Sun, Jun 21, 2026 at 7:53 PM vignesh C <[email protected]> wrote:
> > > >
> > > > While attempting to log a conflict, a concurrent ALTER SUBSCRIPTION
> > > > can change the conflict logging destination from all to log. In this
> > > > scenario, the apply worker may already have cached the conflictlogdest
> > > > information, including the OID of the current conflict log table.
> > > > However, the concurrent ALTER SUBSCRIPTION drops the conflict log
> > > > table as part of the destination change:
> > > > +Relation
> > > > +GetConflictLogDestAndTable(ConflictLogDest *log_dest)
> > > > +{
> > > > + Oid conflictlogrelid;
> > > > +
> > > > + /*
> > > > + * Convert the text log destination to the internal enum.
> > > > MySubscription
> > > > + * already contains the data from pg_subscription.
> > > > + */
> > > > + *log_dest = GetConflictLogDest(MySubscription->conflictlogdest);
> > > > +
> > > > + /* Quick exit if a conflict log table was not requested. */
> > > > + if (!CONFLICTS_LOGGED_TO_TABLE(*log_dest))
> > > > + return NULL;
> > > > +
> > > > + conflictlogrelid = MySubscription->conflictlogrelid;
> > > > +
> > > > + Assert(OidIsValid(conflictlogrelid));
> > > > +
> > > > + return table_open(conflictlogrelid, RowExclusiveLock);
> > > > +}
> > > >
> > > > As a result, when the apply worker later attempts to open the cached
> > > > conflict log table, table_open() fails because the relation has
> > > > already been dropped. This causes the error handling path itself to
> > > > fail before the conflict record can be written to either the conflict
> > > > log table or the server log.
> > > >
> > > > In such cases, the conflict record is effectively lost and is not
> > > > logged anywhere. For example:
> > > > 2026-06-21 19:31:13.592 IST [263598] LOG: logical replication apply
> > > > worker for subscription "sub1" has started
> > > > 2026-06-21 19:32:26.731 IST [263598] ERROR: could not open relation
> > > > with OID 16405
> > > > 2026-06-21 19:32:26.731 IST [263598] CONTEXT: processing remote data
> > > > for replication origin "pg_16404" during message type "INSERT" for
> > > > replication target relation "public.t1" in transaction 698, finished
> > > > at 0/017D39A0
> > > > 2026-06-21 19:32:26.735 IST [263471] LOG: background worker "logical
> > > > replication apply worker" (PID 263598) exited with exit code 1
> > > >
> > > > Ideally, failure to access the conflict log table should not prevent
> > > > the conflict from being reported in the server log. This issue is
> > > > present with the v52 version. I have not yet checked if Amit's recent
> > > > patch posted a few minutes ago at [1] handles this issue.
> > > >
> > >
> > > There are two places in the patch from where we LOG/Insert the
> > > conflict data. First is ReportApplyConflict() where we LOG if the
> > > conflict arises from a non-ERROR path (aka conflicts other
> > > INSERT/UPDATE_EXISTS). In that case, the conflict data will be logged
> > > even when we fail to insert into CLT. Second is the place for
> > > conflicts that arose as ERRORs (aka INSERT/UPDATE_EXISTS), where the
> > > conflict information will be logged along with insert failure as
> > > CONTEXT. Can you please verify your test based on this input and share
> > > your findings and thoughts?
> >
> > The scenario I am testing is an insert_exists conflict.
> > On the publisher:
> > CREATE TABLE t1 (c1 int);
> >
> > On the subscriber:
> > CREATE TABLE t1 (c1 int PRIMARY KEY);
> >
> > Then execute the following on the publisher:
> > INSERT INTO t1 VALUES (10);
> > INSERT INTO t1 VALUES (10);
> >
> > The second insert generates an insert_exists conflict on the
> > subscriber. The conflict is reported and logged through the following
> > call chain:
> > apply_handle_insert
> > -> apply_handle_insert_internal
> > -> ExecSimpleRelationInsert
> > -> CheckAndReportConflict
> > -> ReportApplyConflict
> >
> > Pause execution in ReportApplyConflict() at
> > GetConflictLogDestAndTable(), immediately before opening the conflict
> > log table:
> > ...
> > return table_open(conflictlogrelid, RowExclusiveLock);
> > ...
> >
> > While the apply worker is paused, execute the following command concurrently:
> > ALTER SUBSCRIPTION sub1
> > SET (conflict_log_destination = 'log');
> >
> > This succeeds and drops the conflict log table:
> > NOTICE: dropped conflict log table "pg_conflict.pg_conflict_log_16404"
> > for subscription "sub1"
> > ALTER SUBSCRIPTION
> >
> > At this point, GetConflictLogDestAndTable() has already determined
> > that the conflict should be logged to a table and has cached the
> > corresponding relation OID. However, the concurrent ALTER SUBSCRIPTION
> > has removed that table.
> >
> > When execution resumes, the subsequent table_open() call fails with:
> > 2026-06-22 09:24:53.072 IST [304864] ERROR: could not open relation
> > with OID 16405
> >
> > As a result, conflict processing itself fails before the conflict
> > details can be recorded. The conflict is therefore not logged to the
> > conflict log table and is also not emitted to the server log.
> >
>
>
> I understand this case, but I feel it isn't critical because the table
> is going to be dropped in parallel, so ultimately, all data is lost.
> At-max, we can provide a LOG when table-open fails, indicating that
> the CLT table is dropped concurrently and thus conflict-data cannott
> be logged to table.
>
Instead of adding additional LOG, a simpler fix would be to use
try_table_open() and if the table is dropped, silently just LOG the
conflict and proceed (see attached top-up patch). In general, I agree
that it is not a very critical issue but as the fix is simpler, I
thought it is better to address so that apply worker can continue
instead of erroring out. Having said that, I think we can't handle
each and every corner case where there are some other errors before we
can LOG the conflict, say some OOM or some other error happens during
CheckAndReportConflict.
--
With Regards,
Amit Kapila.
From e5954eb1ef36a98b7dcc31697dd2427070892de3 Mon Sep 17 00:00:00 2001
From: Amit Kapila <[email protected]>
Date: Mon, 22 Jun 2026 12:18:17 +0530
Subject: [PATCH v3] Don't lose a conflict when the conflict log table is
dropped mid-logging
A concurrent ALTER SUBSCRIPTION changing conflict_log_destination can drop the
conflict log table while a conflict is being logged, making table_open() fail
and the conflict lost from both the table and the server log. Open it with
try_table_open() instead and, on NULL, fall back to full server-log reporting.
---
src/backend/replication/logical/conflict.c | 108 +++++++++++++++------
src/include/replication/conflict.h | 3 +-
2 files changed, 82 insertions(+), 29 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index c2c15f055e6..983cdf94cf0 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -319,7 +319,6 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
TupleTableSlot *remoteslot, List *conflicttuples)
{
Relation localrel = relinfo->ri_RelationDesc;
- ConflictLogDest dest;
Relation conflictlogrel;
bool log_dest_table;
bool log_dest_logfile;
@@ -327,13 +326,14 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
pgstat_report_subscription_conflict(MySubscription->oid, type);
/*
- * Get the conflict log destination. Also, (if there is one) return the
- * CLT relation already opened and ready for insertion.
+ * Get the conflict log destinations. Also, if a table is one of the
+ * destinations, return the CLT relation already opened and ready for
+ * insertion. If the table was requested but has been dropped concurrently,
+ * GetConflictLogDestAndTable() returns NULL and reports server-log as the
+ * destination instead, so the conflict is still recorded.
*/
- conflictlogrel = GetConflictLogDestAndTable(&dest);
-
- log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
- log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
+ conflictlogrel = GetConflictLogDestAndTable(&log_dest_table,
+ &log_dest_logfile);
/*
* Prepare the conflict log tuple first when the destination includes the
@@ -466,7 +466,6 @@ void
ProcessPendingConflictLogTuple(void)
{
Relation conflictlogrel;
- ConflictLogDest dest;
/* Nothing to do */
if (MyLogicalRepWorker->conflict_log_tuple == NULL)
@@ -474,23 +473,40 @@ ProcessPendingConflictLogTuple(void)
/*
* Insert the deferred conflict log tuple in its own transaction. A
- * failure here (e.g. the conflict log table was dropped, or an
- * out-of-disk-space error) is treated like any other apply error and
- * raises an ERROR; such failures are expected to be rare and persistent.
- * Callers must therefore have already reported (and cleared) any
- * in-progress apply error before calling this, so that this error does not
- * mask the original one.
+ * failure here (e.g. an out-of-disk-space error) is treated like any other
+ * apply error and raises an ERROR; such failures are expected to be rare
+ * and persistent. Callers must therefore have already reported (and
+ * cleared) any in-progress apply error before calling this, so that this
+ * error does not mask the original one.
*/
StartTransactionCommand();
PushActiveSnapshot(GetTransactionSnapshot());
- /* Open conflict log table and insert the tuple */
- conflictlogrel = GetConflictLogDestAndTable(&dest);
- Assert(conflictlogrel);
+ /* Open the conflict log table and insert the tuple. */
+ conflictlogrel = GetConflictLogDestAndTable(NULL, NULL);
- InsertConflictLogTuple(conflictlogrel);
-
- table_close(conflictlogrel, RowExclusiveLock);
+ if (conflictlogrel != NULL)
+ {
+ InsertConflictLogTuple(conflictlogrel);
+ table_close(conflictlogrel, RowExclusiveLock);
+ }
+ else
+ {
+ /*
+ * The conflict log table was dropped concurrently (e.g. by an ALTER
+ * SUBSCRIPTION that changed conflict_log_destination) after the
+ * conflict was already reported to the server log by
+ * ReportApplyConflict(). Nothing more to do; just discard the prepared
+ * tuple and its context string.
+ */
+ heap_freetuple(MyLogicalRepWorker->conflict_log_tuple);
+ MyLogicalRepWorker->conflict_log_tuple = NULL;
+ if (MyLogicalRepWorker->conflict_log_errcontext)
+ {
+ pfree(MyLogicalRepWorker->conflict_log_errcontext);
+ MyLogicalRepWorker->conflict_log_errcontext = NULL;
+ }
+ }
PopActiveSnapshot();
CommitTransactionCommand();
@@ -531,29 +547,65 @@ InitConflictIndexes(ResultRelInfo *relInfo)
* GetConflictLogDestAndTable
*
* Fetches conflict logging metadata from the cached MySubscription pointer.
- * Sets the destination enum in *log_dest and, if applicable, opens and
- * returns the relation handle for the conflict log table.
+ * Reports whether conflicts should be logged to the table and/or the server
+ * log via *log_dest_table and *log_dest_logfile (either may be NULL if the
+ * caller is not interested), and, if a table is one of the destinations, opens
+ * and returns the relation handle for the conflict log table.
+ *
+ * The table is opened with try_table_open(). If the conflict log table has
+ * been dropped concurrently (e.g. by an ALTER SUBSCRIPTION that changed
+ * conflict_log_destination), NULL is returned and the reported destinations
+ * are adjusted to fall back to the server log (*log_dest_table = false,
+ * *log_dest_logfile = true), so the caller still records the conflict instead
+ * of failing.
*/
Relation
-GetConflictLogDestAndTable(ConflictLogDest *log_dest)
+GetConflictLogDestAndTable(bool *log_dest_table, bool *log_dest_logfile)
{
+ ConflictLogDest dest;
Oid conflictlogrelid;
+ Relation conflictlogrel;
/*
- * Convert the text log destination to the internal enum. MySubscription
- * already contains the data from pg_subscription.
+ * Convert the text log destination to the internal enum and report the
+ * individual destinations to the caller. MySubscription already contains
+ * the data from pg_subscription.
*/
- *log_dest = GetConflictLogDest(MySubscription->conflictlogdest);
+ dest = GetConflictLogDest(MySubscription->conflictlogdest);
+
+ if (log_dest_table)
+ *log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
+ if (log_dest_logfile)
+ *log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
/* Quick exit if a conflict log table was not requested. */
- if (!CONFLICTS_LOGGED_TO_TABLE(*log_dest))
+ if (!CONFLICTS_LOGGED_TO_TABLE(dest))
return NULL;
conflictlogrelid = MySubscription->conflictlogrelid;
Assert(OidIsValid(conflictlogrelid));
- return table_open(conflictlogrelid, RowExclusiveLock);
+ /*
+ * Use try_table_open(): the table may have been dropped concurrently by an
+ * ALTER SUBSCRIPTION that changed conflict_log_destination.
+ */
+ conflictlogrel = try_table_open(conflictlogrelid, RowExclusiveLock);
+
+ /*
+ * If the table is gone, fall back to logging the conflict to the server
+ * log so it is not lost: report the table as no longer a destination and
+ * force server-log reporting.
+ */
+ if (conflictlogrel == NULL)
+ {
+ if (log_dest_table)
+ *log_dest_table = false;
+ if (log_dest_logfile)
+ *log_dest_logfile = true;
+ }
+
+ return conflictlogrel;
}
/*
diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h
index b727cc285e9..760f375d088 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -117,6 +117,7 @@ extern void ReportApplyConflict(EState *estate, ResultRelInfo *relinfo,
List *conflicttuples);
extern void ProcessPendingConflictLogTuple(void);
extern void InitConflictIndexes(ResultRelInfo *relInfo);
-extern Relation GetConflictLogDestAndTable(ConflictLogDest *log_dest);
+extern Relation GetConflictLogDestAndTable(bool *log_dest_table,
+ bool *log_dest_logfile);
extern void InsertConflictLogTuple(Relation conflictlogrel);
#endif
--
2.54.0
Attachments:
[text/plain] v52-3-0001-Don-t-lose-a-conflict-when-the-conflict-log-table.txt (7.9K, 2-v52-3-0001-Don-t-lose-a-conflict-when-the-conflict-log-table.txt)
download | inline diff:
From e5954eb1ef36a98b7dcc31697dd2427070892de3 Mon Sep 17 00:00:00 2001
From: Amit Kapila <[email protected]>
Date: Mon, 22 Jun 2026 12:18:17 +0530
Subject: [PATCH v3] Don't lose a conflict when the conflict log table is
dropped mid-logging
A concurrent ALTER SUBSCRIPTION changing conflict_log_destination can drop the
conflict log table while a conflict is being logged, making table_open() fail
and the conflict lost from both the table and the server log. Open it with
try_table_open() instead and, on NULL, fall back to full server-log reporting.
---
src/backend/replication/logical/conflict.c | 108 +++++++++++++++------
src/include/replication/conflict.h | 3 +-
2 files changed, 82 insertions(+), 29 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index c2c15f055e6..983cdf94cf0 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -319,7 +319,6 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
TupleTableSlot *remoteslot, List *conflicttuples)
{
Relation localrel = relinfo->ri_RelationDesc;
- ConflictLogDest dest;
Relation conflictlogrel;
bool log_dest_table;
bool log_dest_logfile;
@@ -327,13 +326,14 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
pgstat_report_subscription_conflict(MySubscription->oid, type);
/*
- * Get the conflict log destination. Also, (if there is one) return the
- * CLT relation already opened and ready for insertion.
+ * Get the conflict log destinations. Also, if a table is one of the
+ * destinations, return the CLT relation already opened and ready for
+ * insertion. If the table was requested but has been dropped concurrently,
+ * GetConflictLogDestAndTable() returns NULL and reports server-log as the
+ * destination instead, so the conflict is still recorded.
*/
- conflictlogrel = GetConflictLogDestAndTable(&dest);
-
- log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
- log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
+ conflictlogrel = GetConflictLogDestAndTable(&log_dest_table,
+ &log_dest_logfile);
/*
* Prepare the conflict log tuple first when the destination includes the
@@ -466,7 +466,6 @@ void
ProcessPendingConflictLogTuple(void)
{
Relation conflictlogrel;
- ConflictLogDest dest;
/* Nothing to do */
if (MyLogicalRepWorker->conflict_log_tuple == NULL)
@@ -474,23 +473,40 @@ ProcessPendingConflictLogTuple(void)
/*
* Insert the deferred conflict log tuple in its own transaction. A
- * failure here (e.g. the conflict log table was dropped, or an
- * out-of-disk-space error) is treated like any other apply error and
- * raises an ERROR; such failures are expected to be rare and persistent.
- * Callers must therefore have already reported (and cleared) any
- * in-progress apply error before calling this, so that this error does not
- * mask the original one.
+ * failure here (e.g. an out-of-disk-space error) is treated like any other
+ * apply error and raises an ERROR; such failures are expected to be rare
+ * and persistent. Callers must therefore have already reported (and
+ * cleared) any in-progress apply error before calling this, so that this
+ * error does not mask the original one.
*/
StartTransactionCommand();
PushActiveSnapshot(GetTransactionSnapshot());
- /* Open conflict log table and insert the tuple */
- conflictlogrel = GetConflictLogDestAndTable(&dest);
- Assert(conflictlogrel);
+ /* Open the conflict log table and insert the tuple. */
+ conflictlogrel = GetConflictLogDestAndTable(NULL, NULL);
- InsertConflictLogTuple(conflictlogrel);
-
- table_close(conflictlogrel, RowExclusiveLock);
+ if (conflictlogrel != NULL)
+ {
+ InsertConflictLogTuple(conflictlogrel);
+ table_close(conflictlogrel, RowExclusiveLock);
+ }
+ else
+ {
+ /*
+ * The conflict log table was dropped concurrently (e.g. by an ALTER
+ * SUBSCRIPTION that changed conflict_log_destination) after the
+ * conflict was already reported to the server log by
+ * ReportApplyConflict(). Nothing more to do; just discard the prepared
+ * tuple and its context string.
+ */
+ heap_freetuple(MyLogicalRepWorker->conflict_log_tuple);
+ MyLogicalRepWorker->conflict_log_tuple = NULL;
+ if (MyLogicalRepWorker->conflict_log_errcontext)
+ {
+ pfree(MyLogicalRepWorker->conflict_log_errcontext);
+ MyLogicalRepWorker->conflict_log_errcontext = NULL;
+ }
+ }
PopActiveSnapshot();
CommitTransactionCommand();
@@ -531,29 +547,65 @@ InitConflictIndexes(ResultRelInfo *relInfo)
* GetConflictLogDestAndTable
*
* Fetches conflict logging metadata from the cached MySubscription pointer.
- * Sets the destination enum in *log_dest and, if applicable, opens and
- * returns the relation handle for the conflict log table.
+ * Reports whether conflicts should be logged to the table and/or the server
+ * log via *log_dest_table and *log_dest_logfile (either may be NULL if the
+ * caller is not interested), and, if a table is one of the destinations, opens
+ * and returns the relation handle for the conflict log table.
+ *
+ * The table is opened with try_table_open(). If the conflict log table has
+ * been dropped concurrently (e.g. by an ALTER SUBSCRIPTION that changed
+ * conflict_log_destination), NULL is returned and the reported destinations
+ * are adjusted to fall back to the server log (*log_dest_table = false,
+ * *log_dest_logfile = true), so the caller still records the conflict instead
+ * of failing.
*/
Relation
-GetConflictLogDestAndTable(ConflictLogDest *log_dest)
+GetConflictLogDestAndTable(bool *log_dest_table, bool *log_dest_logfile)
{
+ ConflictLogDest dest;
Oid conflictlogrelid;
+ Relation conflictlogrel;
/*
- * Convert the text log destination to the internal enum. MySubscription
- * already contains the data from pg_subscription.
+ * Convert the text log destination to the internal enum and report the
+ * individual destinations to the caller. MySubscription already contains
+ * the data from pg_subscription.
*/
- *log_dest = GetConflictLogDest(MySubscription->conflictlogdest);
+ dest = GetConflictLogDest(MySubscription->conflictlogdest);
+
+ if (log_dest_table)
+ *log_dest_table = CONFLICTS_LOGGED_TO_TABLE(dest);
+ if (log_dest_logfile)
+ *log_dest_logfile = CONFLICTS_LOGGED_TO_LOG(dest);
/* Quick exit if a conflict log table was not requested. */
- if (!CONFLICTS_LOGGED_TO_TABLE(*log_dest))
+ if (!CONFLICTS_LOGGED_TO_TABLE(dest))
return NULL;
conflictlogrelid = MySubscription->conflictlogrelid;
Assert(OidIsValid(conflictlogrelid));
- return table_open(conflictlogrelid, RowExclusiveLock);
+ /*
+ * Use try_table_open(): the table may have been dropped concurrently by an
+ * ALTER SUBSCRIPTION that changed conflict_log_destination.
+ */
+ conflictlogrel = try_table_open(conflictlogrelid, RowExclusiveLock);
+
+ /*
+ * If the table is gone, fall back to logging the conflict to the server
+ * log so it is not lost: report the table as no longer a destination and
+ * force server-log reporting.
+ */
+ if (conflictlogrel == NULL)
+ {
+ if (log_dest_table)
+ *log_dest_table = false;
+ if (log_dest_logfile)
+ *log_dest_logfile = true;
+ }
+
+ return conflictlogrel;
}
/*
diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h
index b727cc285e9..760f375d088 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -117,6 +117,7 @@ extern void ReportApplyConflict(EState *estate, ResultRelInfo *relinfo,
List *conflicttuples);
extern void ProcessPendingConflictLogTuple(void);
extern void InitConflictIndexes(ResultRelInfo *relInfo);
-extern Relation GetConflictLogDestAndTable(ConflictLogDest *log_dest);
+extern Relation GetConflictLogDestAndTable(bool *log_dest_table,
+ bool *log_dest_logfile);
extern void InsertConflictLogTuple(Relation conflictlogrel);
#endif
--
2.54.0
view thread (154+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Proposal: Conflict log history table for Logical Replication
In-Reply-To: <CAA4eK1J+PRbKpPWLNBc6UJBgsUFbnczkwsKAQegEiXmLP=ng0w@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox