| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Peter Smith <smithpb2250(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-11-26 11:18:50 |
| Message-ID: | CAFiTN-s97fgCFwnQQgEMafw=-tB2J9JFyt89mUqWkCqrVzOcxA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Nov 26, 2025 at 4:15 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Wed, Nov 26, 2025 at 2:05 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Tue, Nov 25, 2025 at 4:06 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Nov 25, 2025 at 1:59 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > >
> > > On a separate note, I've been considering how to manage conflict log
> > > insertions when an error causes the outer transaction to abort, which
> > > seems to be a non-trivial.
> > >
> > > Here is what I have in mind:
> > > ======================
> > > First, prepare_conflict_log() would be executed from
> > > ReportApplyConflict(). This function would handle all preliminary
> > > work, such as preparing the tuple for the conflict log table. Second,
> > > insert_conflict_log() would be executed. If the error level in
> > > ReportApplyConflict() is LOG, the insertion would occur directly.
> > > Otherwise, the log information would be stored in a global variable
> > > and inserted in a separate transaction once we exit start_apply() due
> > > to the error.
> > >
> > > @shveta malik @Amit Kapila let me know what you think? Or do you
> > > think it can be simplified?
> > >
> >
> > I could not think of a better way. This idea works for me. I had
> > doubts if it will be okay to start a new transaction in catch-block
> > (if we plan to do it in start_apply's), but then I found few other
> > functions doing it (see do_autovacuum, perform_work_item,
> > _SPI_commit). So IMO, we should be good.
> >
>
> On re-reading, I think you were not suggesting to handle it in the
> CATCH block. Where exactly once we exit start_apply?
> But since the situation will arise only in case of ERROR, I thought
> handling in catch-block could be one option.
Yeah it makes sense to handle in catch block, I have done that in the
attached patch and also handled other comments by Peter.
Now pending work status
1) fixed review comments of 0002 and 0003 - Pending
2) Need to add replica identity tuple instead of full tuple -- Done
3) Keeping the logs in case of outer transaction failure by moving log
insertion outside the main transaction - reported by Shveta - Done
(might need more validation and testing)
4) Run pgindent -- planning to do it after we complete the first level
of review - Pending
5) Subscription test cases for logging the actual conflicts - Pending
--
Regards,
Dilip Kumar
Google
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Add-configurable-conflict-log-table-for-Logical-R.patch | application/octet-stream | 103.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Marcos Pegoraro | 2025-11-26 11:24:57 | Re: Additional info for CREATE ROLE with REPLICATION |
| Previous Message | Daniel Gustafsson | 2025-11-26 11:16:52 | Re: The pgperltidy diffs in HEAD |