Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kqfnj-0003g6-DC for pgsql-bugs@arkaria.postgresql.org; Sat, 19 Dec 2020 17:13:52 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1kqfni-0004Tm-77 for pgsql-bugs@arkaria.postgresql.org; Sat, 19 Dec 2020 17:13:50 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kqfnh-0004Tf-Ub for pgsql-bugs@lists.postgresql.org; Sat, 19 Dec 2020 17:13:50 +0000 Received: from forwardcorp1o.mail.yandex.net ([2a02:6b8:0:1a2d::193]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kqfnb-0002H3-IO for pgsql-bugs@lists.postgresql.org; Sat, 19 Dec 2020 17:13:49 +0000 Received: from myt5-23f0be3aa648.qloud-c.yandex.net (myt5-23f0be3aa648.qloud-c.yandex.net [IPv6:2a02:6b8:c12:3e29:0:640:23f0:be3a]) by forwardcorp1o.mail.yandex.net (Yandex) with ESMTP id 115882E153B for ; Sat, 19 Dec 2020 20:13:41 +0300 (MSK) Received: from myt5-70c90f7d6d7d.qloud-c.yandex.net (myt5-70c90f7d6d7d.qloud-c.yandex.net [2a02:6b8:c12:3e2c:0:640:70c9:f7d]) by myt5-23f0be3aa648.qloud-c.yandex.net (mxbackcorp/Yandex) with ESMTP id FbxcYcb3hs-Dex8bdwY; Sat, 19 Dec 2020 20:13:40 +0300 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex-team.ru; s=default; t=1608398020; bh=mwSQuQrYUnBsKz9XiXwxFrmiybA4mHui8GUPmcyJ2cA=; h=To:Date:Message-Id:Subject:From; b=HL1xM943kBicOvw0hTYS+sEcOFLBtAcU+ciKgd1QUhNqaNjnE7gRk771k7ZahOjGV Zb2MU1dSNV/t9oxvB+ckYZ7gOt/L9DNLQSnMGq3TZC4uMY1fVcKeAC+DfoL261l/g1 mMUgemVvELtXsCKV6A4v+Xyxj+ekDaYKVLYNw/fk= Authentication-Results: myt5-23f0be3aa648.qloud-c.yandex.net; dkim=pass header.i=@yandex-team.ru Received: from dynamic-vpn.dhcp.yndx.net (dynamic-vpn.dhcp.yndx.net [2a02:6b8:b080:7324::1:f]) by myt5-70c90f7d6d7d.qloud-c.yandex.net (smtpcorp/Yandex) with ESMTPSA id eemYGLJ34Z-DenaRS3R; Sat, 19 Dec 2020 20:13:40 +0300 (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client certificate not present) From: Andrey Borodin Content-Type: multipart/mixed; boundary="Apple-Mail=_11B45E86-864C-4E0F-A833-A2D67A1C143A" Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\)) Subject: CREATE INDEX CONCURRENTLY does not index prepared xact's data Message-Id: <2E712143-97F7-4890-B470-4A35142ABC82@yandex-team.ru> Date: Sat, 19 Dec 2020 22:13:39 +0500 To: pgsql-bugs@lists.postgresql.org X-Mailer: Apple Mail (2.3608.120.23.2.4) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --Apple-Mail=_11B45E86-864C-4E0F-A833-A2D67A1C143A Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Hi hackers! $subj. Steps to reproduce: create extension if not exists amcheck; create table if not exists t1(i int); begin; insert into t1 values(1); prepare transaction 'x'; create index concurrently i1 on t1(i);=20 commit prepared 'x'; select bt_index_check('i1', true); I observe: NOTICE: heap tuple (1,8) from table "t1" lacks matching index tuple = within index "i1" I expect: awaiting 'x' commit before index is created, correct index = after. This happens because WaitForLockersMultiple() does not take prepared = xacts into account. Meanwhile CREATE INDEX CONCURRENTLY expects that = locks are dropped only when transaction commit is visible. This issue affects pg_repack and similar machinery based on CIC. PFA draft of a fix. Best regards, Andrey Borodin. --Apple-Mail=_11B45E86-864C-4E0F-A833-A2D67A1C143A Content-Disposition: attachment; filename=v1-0001-Wait-for-prepared-xacts-in-CREATE-INDEX-CONCURREN.patch Content-Type: application/octet-stream; x-unix-mode=0644; name="v1-0001-Wait-for-prepared-xacts-in-CREATE-INDEX-CONCURREN.patch" Content-Transfer-Encoding: quoted-printable =46rom=2002a3baa7f963d5116b3e4506a0f1dcac2c3c16a9=20Mon=20Sep=2017=20= 00:00:00=202001=0AFrom:=20Andrey=20Borodin=20=0ADate:=20= Sat,=2019=20Dec=202020=2021:53:00=20+0500=0ASubject:=20[PATCH=20v1]=20= Wait=20for=20prepared=20xacts=20in=20CREATE=20INDEX=20CONCURRENTLY=20= locks=0A=20infrastructure=0A=0A---=0A=20src/backend/storage/lmgr/lmgr.c=20= |=20=205=20++---=0A=20src/backend/storage/lmgr/lock.c=20|=2033=20= +++++++++++++++++++++------------=0A=20src/include/storage/lock.h=20=20=20= =20=20=20|=20=205=20+++++=0A=203=20files=20changed,=2028=20= insertions(+),=2015=20deletions(-)=0A=0Adiff=20--git=20= a/src/backend/storage/lmgr/lmgr.c=20b/src/backend/storage/lmgr/lmgr.c=0A= index=207409de9405..c1e8e76f41=20100644=0A---=20= a/src/backend/storage/lmgr/lmgr.c=0A+++=20= b/src/backend/storage/lmgr/lmgr.c=0A@@=20-904,8=20+904,7=20@@=20= WaitForLockersMultiple(List=20*locktags,=20LOCKMODE=20lockmode,=20bool=20= progress)=0A=20=0A=20=09/*=0A=20=09=20*=20Note:=20GetLockConflicts()=20= never=20reports=20our=20own=20xid,=20hence=20we=20need=20not=0A-=09=20*=20= check=20for=20that.=20=20Also,=20prepared=20xacts=20are=20not=20= reported,=20which=20is=20fine=0A-=09=20*=20since=20they=20certainly=20= aren't=20going=20to=20do=20anything=20anymore.=0A+=09=20*=20check=20for=20= that.=20=20Also,=20prepared=20xacts=20are=20reported=20and=20awaited.=0A=20= =09=20*/=0A=20=0A=20=09/*=20Finally=20wait=20for=20each=20such=20= transaction=20to=20complete=20*/=0A@@=20-913,7=20+912,7=20@@=20= WaitForLockersMultiple(List=20*locktags,=20LOCKMODE=20lockmode,=20bool=20= progress)=0A=20=09{=0A=20=09=09VirtualTransactionId=20*lockholders=20=3D=20= lfirst(lc);=0A=20=0A-=09=09while=20= (VirtualTransactionIdIsValid(*lockholders))=0A+=09=09while=20= (VirtualTransactionIdIsValidOrPreparedXact(*lockholders))=0A=20=09=09{=0A= =20=09=09=09/*=20If=20requested,=20publish=20who=20we're=20going=20to=20= wait=20for.=20*/=0A=20=09=09=09if=20(progress)=0Adiff=20--git=20= a/src/backend/storage/lmgr/lock.c=20b/src/backend/storage/lmgr/lock.c=0A= index=20d86566f455..9681fcabc8=20100644=0A---=20= a/src/backend/storage/lmgr/lock.c=0A+++=20= b/src/backend/storage/lmgr/lock.c=0A@@=20-2904,9=20+2904,7=20@@=20= FastPathGetRelationLockEntry(LOCALLOCK=20*locallock)=0A=20=20*=20so=20= use=20of=20this=20function=20has=20to=20be=20thought=20about=20= carefully.=0A=20=20*=0A=20=20*=20Note=20we=20never=20include=20the=20= current=20xact's=20vxid=20in=20the=20result=20array,=0A-=20*=20since=20= an=20xact=20never=20blocks=20itself.=20=20Also,=20prepared=20= transactions=20are=0A-=20*=20ignored,=20which=20is=20a=20bit=20more=20= debatable=20but=20is=20appropriate=20for=20current=0A-=20*=20uses=20of=20= the=20result.=0A+=20*=20since=20an=20xact=20never=20blocks=20itself.=0A=20= =20*/=0A=20VirtualTransactionId=20*=0A=20GetLockConflicts(const=20= LOCKTAG=20*locktag,=20LOCKMODE=20lockmode,=20int=20*countp)=0A@@=20= -2931,15=20+2929,17=20@@=20GetLockConflicts(const=20LOCKTAG=20*locktag,=20= LOCKMODE=20lockmode,=20int=20*countp)=0A=20=0A=20=09/*=0A=20=09=20*=20= Allocate=20memory=20to=20store=20results,=20and=20fill=20with=20= InvalidVXID.=20=20We=20only=0A-=09=20*=20need=20enough=20space=20for=20= MaxBackends=20+=20a=20terminator,=20since=20prepared=20xacts=0A-=09=20*=20= don't=20count.=20InHotStandby=20allocate=20once=20in=20TopMemoryContext.=0A= +=09=20*=20need=20enough=20space=20for=20MaxBackends=20+=20= max_prepared_xacts=20+=20a=0A+=09=20*=20terminator.=20InHotStandby=20= allocate=20once=20in=20TopMemoryContext.=0A=20=09=20*/=0A=20=09if=20= (InHotStandby)=0A=20=09{=0A=20=09=09if=20(vxids=20=3D=3D=20NULL)=0A=20=09= =09=09vxids=20=3D=20(VirtualTransactionId=20*)=0A=20=09=09=09=09= MemoryContextAlloc(TopMemoryContext,=0A-=09=09=09=09=09=09=09=09=20=20=20= sizeof(VirtualTransactionId)=20*=20(MaxBackends=20+=201));=0A+=09=09=09=09= =09=09=09=09=20=20=20sizeof(VirtualTransactionId)=20*=20(MaxBackends=20=0A= +=09=09=09=09=09=09=09=09=20=20=20+=20max_prepared_xacts=0A+=09=09=09=09=09= =09=09=09=20=20=20+=201));=0A=20=09}=0A=20=09else=0A=20=09=09vxids=20=3D=20= (VirtualTransactionId=20*)=0A@@=20-3085,10=20+3085,10=20@@=20= GetLockConflicts(const=20LOCKTAG=20*locktag,=20LOCKMODE=20lockmode,=20= int=20*countp)=0A=20=0A=20=09=09=09=09/*=0A=20=09=09=09=09=20*=20If=20we=20= see=20an=20invalid=20VXID,=20then=20either=20the=20xact=20has=20already=0A= -=09=09=09=09=20*=20committed=20(or=20aborted),=20or=20it's=20a=20= prepared=20xact.=20=20In=20either=0A-=09=09=09=09=20*=20case=20we=20may=20= ignore=20it.=0A+=09=09=09=09=20*=20committed=20(or=20aborted),=20or=20= it's=20a=20prepared=20xact.=20We=20take=0A+=09=09=09=09=20*=20prepared=20= xacts=20into=20account=20too.=0A=20=09=09=09=09=20*/=0A-=09=09=09=09if=20= (VirtualTransactionIdIsValid(vxid))=0A+=09=09=09=09if=20= (VirtualTransactionIdIsValidOrPreparedXact(vxid))=0A=20=09=09=09=09{=0A=20= =09=09=09=09=09int=09=09=09i;=0A=20=0A@@=20-3108,7=20+3108,7=20@@=20= GetLockConflicts(const=20LOCKTAG=20*locktag,=20LOCKMODE=20lockmode,=20= int=20*countp)=0A=20=0A=20=09LWLockRelease(partitionLock);=0A=20=0A-=09= if=20(count=20>=20MaxBackends)=09/*=20should=20never=20happen=20*/=0A+=09= if=20(count=20>=20MaxBackends=20+=20max_prepared_xacts)=09/*=20should=20= never=20happen=20*/=0A=20=09=09elog(PANIC,=20"too=20many=20conflicting=20= locks=20found");=0A=20=0A=20=09vxids[count].backendId=20=3D=20= InvalidBackendId;=0A@@=20-4461,9=20+4461,18=20@@=20bool=0A=20= VirtualXactLock(VirtualTransactionId=20vxid,=20bool=20wait)=0A=20{=0A=20=09= LOCKTAG=09=09tag;=0A-=09PGPROC=09=20=20=20*proc;=0A+=09PGPROC=09=20=20=20= *proc=20=3D=20NULL;=0A=20=0A-=09= Assert(VirtualTransactionIdIsValid(vxid));=0A+=09= Assert(VirtualTransactionIdIsValidOrPreparedXact(vxid));=0A+=0A+=09if=20= (VirtualTransactionIdIsPreparedXact(vxid))=0A+=09{=0A+=09=09/*=20If=20= it's=20prepared=20xact=20-=20just=20wait=20for=20it=20*/=0A+=09=09= SET_LOCKTAG_TRANSACTION(tag,=20vxid.localTransactionId);=0A+=09=09(void)=20= LockAcquire(&tag,=20ShareLock,=20false,=20false);=0A+=09=09= LockRelease(&tag,=20ShareLock,=20false);=0A+=09=09return=20true;=0A+=09}=0A= =20=0A=20=09SET_LOCKTAG_VIRTUALTRANSACTION(tag,=20vxid);=0A=20=0Adiff=20= --git=20a/src/include/storage/lock.h=20b/src/include/storage/lock.h=0A= index=201c3e9c1999..dbd2a32678=20100644=0A---=20= a/src/include/storage/lock.h=0A+++=20b/src/include/storage/lock.h=0A@@=20= -70,6=20+70,11=20@@=20typedef=20struct=0A=20#define=20= VirtualTransactionIdIsValid(vxid)=20\=0A=20=09(((vxid).backendId=20!=3D=20= InvalidBackendId)=20&&=20\=0A=20=09=20= LocalTransactionIdIsValid((vxid).localTransactionId))=0A+#define=20= VirtualTransactionIdIsValidOrPreparedXact(vxid)=20\=0A+=09= (((vxid).backendId=20=3D=3D=20InvalidBackendId)=20||=20\=0A+=09=20= LocalTransactionIdIsValid((vxid).localTransactionId))=0A+#define=20= VirtualTransactionIdIsPreparedXact(vxid)=20\=0A+=09((vxid).backendId=20= =3D=3D=20InvalidBackendId)=0A=20#define=20= VirtualTransactionIdEquals(vxid1,=20vxid2)=20\=0A=20=09= ((vxid1).backendId=20=3D=3D=20(vxid2).backendId=20&&=20\=0A=20=09=20= (vxid1).localTransactionId=20=3D=3D=20(vxid2).localTransactionId)=0A--=20= =0A2.24.3=20(Apple=20Git-128)=0A=0A= --Apple-Mail=_11B45E86-864C-4E0F-A833-A2D67A1C143A--