Re: Adding REPACK [concurrently]

On 2026-Apr-07, Antonin Houska wrote:

> > @@ -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?

Yeah, he did.

> 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?

Yeah, apparently it keeps track of tainted bytes somehow. Clever.

The change to palloc0() that I was proposing did not fix the problem,
because the stack allocated struct overwrote those zeroes with the
uninitialized padding bytes.

I ended up with an equivalent fix to Srinath's -- zero-initializing
the stack-allocated struct, so that the bytes that end up copied by
memcpy() are all defined. Srinath confirmed that in his environment the
valgrind failure goes away, so I think we're good.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"No tengo por qué estar de acuerdo con lo que pienso"
(Carlos Caszeli)