Horton
Menu

Re: Adding REPACK [concurrently]

Srinath Reddy Sadipiralla <srinath2133@gmail.com>

View thread
From: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
To: Antonin Houska <ah@cybertec.at>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, Noah Misch <noah@leadboat.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, vignesh C <vignesh21@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Mihail Nikalayeu <mihailnikalayeu@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>, Robert Treat <rob@xzilla.net>
Date: 2026-04-07T09:41:17Z
Lists: pgsql-hackers 
On Tue, Apr 7, 2026 at 2:59 PM Antonin Houska <ah@cybertec.at> wrote:

> Antonin Houska <ah@cybertec.at> wrote:
>
> > Srinath Reddy Sadipiralla <srinath2133@gmail.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.
>

yeah , same in my environment aslo

==00:00:00:06.569 3479320== Syscall param pwrite64(buf) points to
uninitialised byte(s)
==00:00:00:06.569 3479320==    at 0x55D6D38: pwrite (pwrite64.c:25)
==00:00:00:06.569 3479320==    by 0x842EE7: pg_pwritev (pg_iovec.h:101)
==00:00:00:06.569 3479320==    by 0x845F67: FileWriteV (fd.c:2280)
==00:00:00:06.569 3479320==    by 0x840877: FileWrite (fd.h:245)
==00:00:00:06.569 3479320==    by 0x841407: BufFileDumpBuffer
(buffile.c:538)
==00:00:00:06.569 3479320==    by 0x841A67: BufFileFlush (buffile.c:724)
==00:00:00:06.569 3479320==    by 0x8410B7: BufFileClose (buffile.c:418)
==00:00:00:06.569 3479320==    by 0x4BFBBB: export_initial_snapshot
(repack_worker.c:346)
==00:00:00:06.569 3479320==    by 0x4BF703: RepackWorkerMain
(repack_worker.c:145)
==00:00:00:06.569 3479320==    by 0x765D3F: BackgroundWorkerMain
(bgworker.c:865)
==00:00:00:06.569 3479320==    by 0x76C703: postmaster_child_launch
(launch_backend.c:265)
==00:00:00:06.569 3479320==    by 0x774F87: StartBackgroundWorker
(postmaster.c:4197)
==00:00:00:06.569 3479320==  Address 0x7b055f2 is 106 bytes inside a block
of size 8,272 client-defined
==00:00:00:06.569 3479320==    at 0xB234F4: palloc (mcxt.c:1411)
==00:00:00:06.569 3479320==    by 0x8408BF: makeBufFileCommon
(buffile.c:121)
==00:00:00:06.569 3479320==    by 0x840C2F: BufFileCreateFileSet
(buffile.c:272)
==00:00:00:06.569 3479320==    by 0x4BFB7F: export_initial_snapshot
(repack_worker.c:341)
==00:00:00:06.569 3479320==    by 0x4BF703: RepackWorkerMain
(repack_worker.c:145)
==00:00:00:06.569 3479320==    by 0x765D3F: BackgroundWorkerMain
(bgworker.c:865)
==00:00:00:06.569 3479320==    by 0x76C703: postmaster_child_launch
(launch_backend.c:265)
==00:00:00:06.569 3479320==    by 0x774F87: StartBackgroundWorker
(postmaster.c:4197)
==00:00:00:06.569 3479320==    by 0x775277: maybe_start_bgworkers
(postmaster.c:4362)
==00:00:00:06.569 3479320==    by 0x7739F7:
LaunchMissingBackgroundProcesses (postmaster.c:3437)
==00:00:00:06.569 3479320==    by 0x770C2B: ServerLoop (postmaster.c:1737)
==00:00:00:06.569 3479320==    by 0x77037B: PostmasterMain
(postmaster.c:1412)
==00:00:00:06.569 3479320==  Uninitialised value was created by a stack
allocation
==00:00:00:06.569 3479320==    at 0xB43008: SerializeSnapshot
(snapmgr.c:1737)
==00:00:00:06.569 3479320==

and you are spot on here with the explanation with offsets

Size snap_size; 8 bytes;
TransactionId xmin; 4 bytes
TransactionId xmax; 4 bytes
uint32 xcnt; 4 bytes
int32 subxcnt; 4 bytes
bool suboverflowed; 1 byte
bool takenDuringRecovery; 1 byte
/* 2 bytes of padding */
CommandId curcid; 4 bytes


-- 
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow old WAL recycling during REPACK CONCURRENTLY

  2. Advance restart_lsn more eagerly in LogicalConfirmReceivedLocation

  3. Remove unnecessary signal handler change

  4. Improve REPACK (CONCURRENTLY) error messages some more

  5. Revert "Allow logical replication snapshots to be database-specific"

  6. Move REPACK (CONCURRENTLY) test out of stock regression tests

  7. REPACK: do not require REPLICATION or LOGIN

  8. Add missing initialization

  9. Simplify declaration of memcpy target

  10. Reserve replication slots specifically for REPACK

  11. doc: Add an example of REPACK (CONCURRENTLY)

  12. Allow logical replication snapshots to be database-specific

  13. Avoid different-size pointer-to-integer cast

  14. Fix valgrind failure

  15. Add CONCURRENTLY option to REPACK

  16. Rename cluster.c to repack.c (and corresponding .h)

  17. Allow index_create to suppress index_build progress reporting

  18. Make index_concurrently_create_copy more general

  19. Document the 'command' column of pg_stat_progress_repack

  20. Introduce the REPACK command

  21. Toggle logical decoding dynamically based on logical slot presence.

  22. Split vacuumdb to create vacuuming.c/h

  23. Remove ReorderBufferTupleBuf structure.

  24. Revert changes to CONCURRENTLY that "sped up" Xmin advance

  25. VACUUM: ignore indexing operations with CONCURRENTLY