Re: pgsql: Generational memory allocator - Mailing list pgsql-committers

From Tom Lane
Subject Re: pgsql: Generational memory allocator
Date
Msg-id 4618.1511573138@sss.pgh.pa.us
Whole thread Raw
In response to Re: pgsql: Generational memory allocator  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pgsql: Generational memory allocator  (Tomas Vondra <tv@fuzzy.cz>)
Re: pgsql: Generational memory allocator  (Simon Riggs <simon@2ndquadrant.com>)
List pgsql-committers
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

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/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 != InvalidCommandId &&
--- 783,799 ----
       */
      ReorderBufferXidSetCatalogChanges(builder->reorder, xid, lsn);

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

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

pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Improve valgrind logic in aset.c,and fix multiple issues in gen
Next
From: Tomas Vondra
Date:
Subject: Re: pgsql: Generational memory allocator