Thread: pgsql: Assert(IsTransactionState()) in RelationIdGetRelation().
Assert(IsTransactionState()) in RelationIdGetRelation(). Commit 42c80c696e9c8323841180029cc62741c21bd356 added an Assert(IsTransactionState()) in SearchCatCache(), to catch any code that thought it could do a catcache lookup outside transactions. Extend the same idea to relcache lookups. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/ddfc9cb054abed4d08cc2709c9b2197dab96f449 Modified Files -------------- src/backend/utils/cache/relcache.c | 3 +++ 1 file changed, 3 insertions(+)
On 2014-02-06 16:28:23 +0000, Tom Lane wrote:
> Assert(IsTransactionState()) in RelationIdGetRelation().
>
> Commit 42c80c696e9c8323841180029cc62741c21bd356 added an
> Assert(IsTransactionState()) in SearchCatCache(), to catch
> any code that thought it could do a catcache lookup outside
> transactions. Extend the same idea to relcache lookups.
Hm, I am not sure if it works this way. In a patched postgres I just
got:
#2 0x00000000008b4b27 in ExceptionalCondition (conditionName=0xa90a78 "!(IsTransactionState())", errorType=0xa90708
"FailedAssertion",
fileName=0xa905c8 "/home/andres/src/postgresql/src/backend/utils/cache/relcache.c", lineNumber=1622)
at /home/andres/src/postgresql/src/backend/utils/error/assert.c:54
#3 0x00000000008a3fd6 in RelationIdGetRelation (relationId=1259) at
/home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1622
#4 0x00000000004a25ea in relation_open (relationId=1259, lockmode=1) at
/home/andres/src/postgresql/src/backend/access/heap/heapam.c:1038
#5 0x00000000004a2898 in heap_open (relationId=1259, lockmode=1) at
/home/andres/src/postgresql/src/backend/access/heap/heapam.c:1201
#6 0x00000000008a10ba in ScanPgRelation (targetRelId=2693, indexOK=1 '\001', suspend_snap=1 '\001')
at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:308
#7 0x00000000008a268c in RelationInitPhysicalAddr (relation=0x7fac2a842658)
at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1019
#8 0x00000000008a4861 in RelationClearRelation (relation=0x7fac2a842658, rebuild=1 '\001')
at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1949
#9 0x00000000008a4db5 in RelationFlushRelation (relation=0x7fac2a842658)
at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:2157
#10 0x00000000008a4ec7 in RelationCacheInvalidateEntry (relationId=2693)
at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:2209
#11 0x000000000089d7dc in LocalExecuteInvalidationMessage (msg=0xd53df0 <messages.8525+240>)
at /home/andres/src/postgresql/src/backend/utils/cache/inval.c:546
#12 0x000000000076fdc5 in ReceiveSharedInvalidMessages (invalFunction=0x89d6ef <LocalExecuteInvalidationMessage>,
resetFunction=0x89d930 <InvalidateSystemCaches>) at
/home/andres/src/postgresql/src/backend/storage/ipc/sinval.c:127
#13 0x000000000089d9f9 in AcceptInvalidationMessages () at
/home/andres/src/postgresql/src/backend/utils/cache/inval.c:640
#14 0x00000000004f12e1 in AtStart_Cache () at /home/andres/src/postgresql/src/backend/access/transam/xact.c:855
#15 0x00000000004f247c in StartTransaction () at /home/andres/src/postgresql/src/backend/access/transam/xact.c:1834
#16 0x00000000004f2f7a in StartTransactionCommand () at
/home/andres/src/postgresql/src/backend/access/transam/xact.c:2507
#17 0x0000000000737827 in ReorderBufferCommit (rb=0x15cc568, xid=3170, commit_lsn=71557144, end_lsn=71557704,
commit_time=445461926831965)
at /home/andres/src/postgresql/src/backend/replication/logical/reorderbuffer.c:1353
Obviously this isn't reproducable in core postgres the way it is here,
but it looks like it'd be possible:
static void
StartTransaction(void)
{
...
AtStart_GUC();
AtStart_Inval();
AtStart_Cache();
AfterTriggerBeginXact();
/*
* done with start processing, set current transaction state to "in
* progress"
*/
s->state = TRANS_INPROGRESS;
}
Not immediately sure how to handle this, except adding another state test
function allowing TRANS_START|INPROGRESS|COMMIT?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-02-06 16:28:23 +0000, Tom Lane wrote:
>> Assert(IsTransactionState()) in RelationIdGetRelation().
> Hm, I am not sure if it works this way. In a patched postgres I just
> got:
> #2 0x00000000008b4b27 in ExceptionalCondition (conditionName=0xa90a78 "!(IsTransactionState())", errorType=0xa90708
"FailedAssertion",
> fileName=0xa905c8 "/home/andres/src/postgresql/src/backend/utils/cache/relcache.c", lineNumber=1622)
> at /home/andres/src/postgresql/src/backend/utils/error/assert.c:54
> #3 0x00000000008a3fd6 in RelationIdGetRelation (relationId=1259) at
/home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1622
> #4 0x00000000004a25ea in relation_open (relationId=1259, lockmode=1) at
/home/andres/src/postgresql/src/backend/access/heap/heapam.c:1038
> #5 0x00000000004a2898 in heap_open (relationId=1259, lockmode=1) at
/home/andres/src/postgresql/src/backend/access/heap/heapam.c:1201
> #6 0x00000000008a10ba in ScanPgRelation (targetRelId=2693, indexOK=1 '\001', suspend_snap=1 '\001')
> at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:308
> #7 0x00000000008a268c in RelationInitPhysicalAddr (relation=0x7fac2a842658)
> at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1019
Um ... what is RelationInitPhysicalAddr doing calling ScanPgRelation?
That function shouldn't be doing any new catalog access.
regards, tom lane
On 2014-02-11 14:45:00 -0500, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-02-06 16:28:23 +0000, Tom Lane wrote: > >> Assert(IsTransactionState()) in RelationIdGetRelation(). > > > Hm, I am not sure if it works this way. In a patched postgres I just > > got: > > #2 0x00000000008b4b27 in ExceptionalCondition (conditionName=0xa90a78 "!(IsTransactionState())", errorType=0xa90708"FailedAssertion", > > fileName=0xa905c8 "/home/andres/src/postgresql/src/backend/utils/cache/relcache.c", lineNumber=1622) > > at /home/andres/src/postgresql/src/backend/utils/error/assert.c:54 > > #3 0x00000000008a3fd6 in RelationIdGetRelation (relationId=1259) at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1622 > > #4 0x00000000004a25ea in relation_open (relationId=1259, lockmode=1) at /home/andres/src/postgresql/src/backend/access/heap/heapam.c:1038 > > #5 0x00000000004a2898 in heap_open (relationId=1259, lockmode=1) at /home/andres/src/postgresql/src/backend/access/heap/heapam.c:1201 > > #6 0x00000000008a10ba in ScanPgRelation (targetRelId=2693, indexOK=1 '\001', suspend_snap=1 '\001') > > at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:308 > > #7 0x00000000008a268c in RelationInitPhysicalAddr (relation=0x7fac2a842658) > > at /home/andres/src/postgresql/src/backend/utils/cache/relcache.c:1019 > > Um ... what is RelationInitPhysicalAddr doing calling ScanPgRelation? > That function shouldn't be doing any new catalog access. Argh, sorry, didn't think far enough. Yes, that's specific to the branch I am testing, and the after your recent changes there shouldn't be any catalog accesses in TRANS_START due to invalidations in master. This just needed to be adapted to that (i.e the same if (IsTransactionState()) added). The reason we're doing catalog accesses there is that when decoding the WAL's contents we want a relation descriptor that looks like one would have when the WAL record was created, but for catalog tables we want the the relfilenode to point to the current relfilenode (normal relations are never accessed). So we use a "historical" snapshot to build the relcache entry and an up2date one to lookup the relfilenode. RelationInitPhysicalAddr() seems like the correct place to deal with that since nothing else needs to know. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services