RE: BUG #18055: logical decoding core on AllocateSnapshotBuilder()

On Tuesday, August 22, 2023 2:19 AM Andres Freund <andres@anarazel.de> wrote:
> 
> Hi,
> 
> On 2023-08-21 21:41:46 +0900, Masahiko Sawada wrote:
> > On Mon, Aug 21, 2023 at 6:05 AM Andres Freund <andres@anarazel.de>
> wrote:
> > >
> > > Hi,
> > >
> > >
> > > I think the real problem is that 272248a0c added InitialRunningXacts
> > > a global variable. If it just lived in in struct SnapBuild, this
> > > whole thing wouldn't be an issue? The commit justified this choice
> > > with avoiding an ABI breakage - but isn't that bogus? The struct is
> > > private to snapbuild.c. It doesn't live on disk (that's SnapBuildOnDisk).
> >
> > No, since SnapBuildOnDisk contains SnapBuild, if we add something to
> > SnapBuild, the on-disk format compatibility would break. See:
> 
> Err, yes. That was a brainfart on my part. But we could still just have handled that
> via the 'version' field.

I thought you mean we can add the InitialRunningXacts into the SnapBuild struct
while avoid serializing them to the disk, so that the disk file size doesn't
change and the reported problem also get fixed.

I think, normally, we need to increment the snapbuild version when changing the
struct size, but this sounds a bit complex as we need to increment the number in all
support back-branches which have different SNAPBUILD_VERSION.

Or if we agree we don't change the version number in this case as we don't plan
to change the disk file size(only change the struct size), it seems work,
although it looks a bit hacky.

Best Regards,
Hou zj