Re: Error "initial slot snapshot too large" in create replication slot

On Mon, Oct 11, 2021 at 4:29 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Mon, 11 Oct 2021 11:49:41 +0530, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote in
> > While creating an "export snapshot" I don't see any protection why the
> > number of xids in the snapshot can not cross the
> > "GetMaxSnapshotXidCount()"?.
> >
> > Basically, while converting the HISTORIC snapshot to the MVCC snapshot
> > in "SnapBuildInitialSnapshot()", we add all the xids between
> > snap->xmin to snap->xmax to the MVCC snap->xip array (xids for which
> > commit were not recorded). The problem is that we add both topxids as
> > well as the subxids into the same array and expect that the "xid"
> > count does not cross the "GetMaxSnapshotXidCount()". So it seems like
> > an issue but I am not sure what is the fix for this, some options are
>
> It seems to me that it is a compromise between the restriction of the
> legitimate snapshot and snapshots created by snapbuild. If the xids
> overflow, the resulting snapshot may lose a siginificant xid, i.e, a
> top-level xid.
>
> > a) Don't limit the xid count in the exported snapshot and dynamically
> > resize the array b) Increase the limit to GetMaxSnapshotXidCount() +
> > GetMaxSnapshotSubxidCount(). But in option b) there would still be a
> > problem that how do we handle the overflowed subtransaction?
>
> I'm afraid that we shouldn't expand the size limits. If I understand
> it correctly, we only need the top-level xids in the exported snapshot

But since we are using this as an MVCC snapshot, if we don't have the
subxid and if we also don't mark the "suboverflowed" flag then IMHO
the sub-transaction visibility might be wrong, Am I missing something?

> and reorder buffer knows whether a xid is a top-level or not after
> establishing a consistent snapshot.
>
> The attached diff tries to make SnapBuildInitialSnapshot exclude
> subtransaction from generated snapshots. It seems working fine for
> you repro. (Of course, I'm not confident that it is the correct thing,
> though..)
>
> What do you think about this?

If your statement that we only need top-xids in the exported snapshot,
is true then this fix looks fine to me. If not then we might need to
add the sub-xids in the snapshot->subxip array and if it crosses the
limit of GetMaxSnapshotSubxidCount(), then we can mark "suboverflowed"
flag.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com