Re: logical replication of truncate command with trigger causes Assert

On Wed, Jun 9, 2021 at 5:29 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> writes:
> > On Jun 8, 2021, at 3:55 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I suppose that either apply_dispatch or LogicalRepApplyLoop needs to
> >> grow some more snapshot management logic, but I've not looked at that
> >> code much, so I don't have an opinion on just where to add it.
>
> > I was looking at those for other reasons prior to hitting this bug.
>
> After looking at it a bit, I see a couple of options:
>
> 1. Just wrap the call of ExecuteTruncateGuts with
> PushActiveSnapshot(GetTransactionSnapshot()) and PopActiveSnapshot().
>
> 2. Decide that we ought to ensure that a snapshot exists throughout
> most of this code. It's not entirely obvious to me that there is no
> code path reachable from, say, apply_handle_truncate's collection of
> relation OIDs that needs a snapshot. If we went for that, I'd think
> the right solution is to do PushActiveSnapshot right after each
> ensure_transaction call, and then PopActiveSnapshot on the way out of
> the respective subroutine. We could then drop the snapshot management
> calls that are currently associated with the executor state.
>

+1 for the second option as with that, apart from what you said it
will take off some load from future developers to decide which part of
changes should be after acquiring snapshot.

--
With Regards,
Amit Kapila.