Re: Proposal: Conflict log history table for Logical Replication

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.

>
> > 4) Is the condition remote_commit_ts > 0 done intentionally?
> > + if (remote_commit_ts > 0)
> > + values[attno++] = TimestampTzGetDatum(remote_commit_ts);
> > + else
> > + nulls[attno++] = true;
> >
> > As I had seen some negative values for certain timestamps. Shouldn't
> > the check be != 0?
> > SELECT extract(epoch FROM '1969-12-31 23:59:59+00'::timestamptz);
> > extract
> > -----------
> > -1.000000
> > (1 row)
>
> I think the 0 can also be generated for timestamptz, but since we are
> initializing `timestamptz` with 0, checking it seems correct.= 0, but
> I need to put more thought into this.
>

I think either way (>0 or !=0) are fine as both will serve the desired
purpose but I think !=0 will be more robust because we are using 0 as
sentinel value for remote_commit_ts.

--
With Regards,
Amit Kapila.