From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Tomas Vondra <tv@fuzzy.cz>, Simon Riggs <simon@2ndquadrant.com>,
        pgsql-committers <pgsql-committers@postgresql.org>
Subject: Re: pgsql: Generational memory allocator
In-reply-to: <16449.1511548313@sss.pgh.pa.us>
References: <E1eHa4J-0006hI-Q8@gemulon.postgresql.org>
 <20171123001649.y2nf7422iay6cw3v@alap3.anarazel.de>
 <CANP8+jKoT=Pg5W_3FcKiDf+FMrJhFYacxt_g5E0=tkmb5s8M6g@mail.gmail.com>
 <c903f275-2150-fa52-64bf-dca7b53ebf8d@fuzzy.cz>
 <16449.1511548313@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Fri, 24 Nov 2017 13:31:53 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <4584.1511573108.0@sss.pgh.pa.us>
Date: Fri, 24 Nov 2017 20:25:38 -0500
Message-ID: <4618.1511573138@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4584.1511573108.1@sss.pgh.pa.us>

I wrote:
> For me, this patch fixes the valgrind failures inside generation.c
> itself, but I still see one more in the test_decoding run: ...
> Not sure what to make of this: the stack traces make it look unrelated
> to the GenerationContext changes, but if it's not related, how come
> skink was passing before that patch went in?

I've pushed fixes for everything that I could find wrong in generation.c
(and there was a lot :-().  But I'm still seeing the "invalid read in
SnapBuildProcessNewCid" failure when I run test_decoding under valgrind.
Somebody who has more familiarity with the logical decoding stuff than
I do needs to look into that.

I tried to narrow down exactly which fetch in SnapBuildProcessNewCid was
triggering the failure, with the attached patch.  Weirdly, *it does not
fail* with this.  I have no explanation for that.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="hack-to-split-up-fetches.patch";
	charset="us-ascii"
Content-ID: <4584.1511573108.2@sss.pgh.pa.us>
Content-Description: hack-to-split-up-fetches.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/rep=
lication/logical/snapbuild.c
index ad65b98..a48db7e 100644
*** a/src/backend/replication/logical/snapbuild.c
--- b/src/backend/replication/logical/snapbuild.c
*************** SnapBuildProcessNewCid(SnapBuild *builde
*** 770,775 ****
--- 770,781 ----
  					   XLogRecPtr lsn, xl_heap_new_cid *xlrec)
  {
  	CommandId	cid;
+ 	TransactionId top_xid;
+ 	RelFileNode node;
+ 	ItemPointerData tid;
+ 	CommandId cmin;
+ 	CommandId cmax;
+ 	CommandId combocid;
  =

  	/*
  	 * we only log new_cid's if a catalog tuple was modified, so mark the
*************** SnapBuildProcessNewCid(SnapBuild *builde
*** 777,786 ****
  	 */
  	ReorderBufferXidSetCatalogChanges(builder->reorder, xid, lsn);
  =

! 	ReorderBufferAddNewTupleCids(builder->reorder, xlrec->top_xid, lsn,
! 								 xlrec->target_node, xlrec->target_tid,
! 								 xlrec->cmin, xlrec->cmax,
! 								 xlrec->combocid);
  =

  	/* figure out new command id */
  	if (xlrec->cmin !=3D InvalidCommandId &&
--- 783,799 ----
  	 */
  	ReorderBufferXidSetCatalogChanges(builder->reorder, xid, lsn);
  =

! 	top_xid =3D xlrec->top_xid;
! 	node =3D xlrec->target_node;
! 	tid =3D xlrec->target_tid;
! 	cmin =3D xlrec->cmin;
! 	cmax =3D xlrec->cmax;
! 	combocid =3D xlrec->combocid;
! =

! 	ReorderBufferAddNewTupleCids(builder->reorder, top_xid, lsn,
! 								 node, tid,
! 								 cmin, cmax,
! 								 combocid);
  =

  	/* figure out new command id */
  	if (xlrec->cmin !=3D InvalidCommandId &&

------- =_aaaaaaaaaa0--