Re: Proposal: Conflict log history table for Logical Replication

On Wed, 24 Jun 2026 at 16:32, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Jun 24, 2026 at 12:53 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Tue, Jun 23, 2026 at 2:22 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > >
> > > Few comments:
> > > 1) Currently we are storing these in shared memory. Looking at the
> > > implementation, these fields are purely worker-private state used to
> > > ferry data across the error boundary from prepare_conflict_log_tuple()
> > > (inside the PG_TRY block) to ProcessPendingConflictLogTuple() (inside
> > > the PG_CATCH block).
>
> Good point.
>
> If it is not required by another process, should
> > > it be moved out of shared memory.
> > > + /* A conflict log tuple that is prepared but not yet inserted. */
> > > + HeapTuple conflict_log_tuple;
> > > +
> > > + /*
> > > + * Error-context string describing the conflict above, used to
> > > annotate any
> > > + * error raised while inserting conflict_log_tuple into the conflict log
> > > + * table. Allocated, like conflict_log_tuple, in ApplyContext.
> > > + */
> > > + char *conflict_log_errcontext;
> >
> > Yeah there is no need for them to be in shared memory, but do we have
> > any other data sturcture where these fits naturally, or we can make
> > them global variables?
> >
>
> Or we can have a file local struct PendingConflictLogData similar to
> FlushPosition. See the attached top-up patch. As the comment
> ("Allocated, like conflict_log_tuple, in ApplyContext") says it is
> allocated in process-local Apply context, it is not safe to keep them
> in shared memory.

These approach looks good, couple of minor comments can be done while
merging the patch:
1) Generally we keep the typedef name and struct name same:
+typedef struct PendingConflictLogData
+{
+ HeapTuple tuple; /* prepared,
not-yet-inserted conflict tuple */
+ char *errcontext_str; /* conflict description for
error context */
+} PendingConflictLog;

2) PendingConflictLog should be included in typedefs.list

Regards,
Vignesh