Re: Adding REPACK [concurrently]

Antonin Houska <ah(at)cybertec(dot)at> wrote:

> Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com> wrote:
>
> > i looked into this , it seems like valgrind catches the uninitialised padding bytes, which
> > repack worker is writing using BufFileWrite, it seems this fix solved the problem.
> >
> > diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
> > index 2e6197f5f35..f5682b87626 100644
> > --- a/src/backend/utils/time/snapmgr.c
> > +++ b/src/backend/utils/time/snapmgr.c
> > @@ -1739,6 +1739,8 @@ SerializeSnapshot(Snapshot snapshot, char *start_address)
> >
> > Assert(snapshot->subxcnt >= 0);
> >
> > + MemSet(&serialized_snapshot, 0, sizeof(SerializedSnapshotData));
> > +
> > /* Copy all required fields */
> > serialized_snapshot.xmin = snapshot->xmin;
> > serialized_snapshot.xmax = snapshot->xmax;
> >
> > thoughts?
>
> Could you reproduce the failure in your environment?
>
> I haven't thought of this explanation because BufFileWrite() only copies the
> data to a buffer in the BufFile structure and BufFileDumpBuffer() writes the
> buffer. Maybe valgrind is able to track the copying?

Given this message, you may be right:

==1617044== Address 0x12d745e2 is 106 bytes inside a block of size 8,272 client-defined

In my environment, the 'buffer' field starts at offset 80 into the BufFile
structure. We first write 8 bytes into it

BufFileWrite(file, &snap_size, sizeof(snap_size));

followed by the snapshot. Since sizeof(SerializedSnapshotData) is 24, the
offset 106 should be the padding following the 'takenDuringRecovery' field.

--
Antonin Houska
Web: https://www.cybertec-postgresql.com