Re: trap instead of error on 32 TiB table - Mailing list pgsql-hackers

From Tom Lane
Subject Re: trap instead of error on 32 TiB table
Date
Msg-id 3606135.1631197527@sss.pgh.pa.us
Whole thread Raw
In response to Re: trap instead of error on 32 TiB table  (Christoph Berg <christoph.berg@credativ.de>)
Responses Re: trap instead of error on 32 TiB table
List pgsql-hackers
Christoph Berg <christoph.berg@credativ.de> writes:
>> Can you provide a stack trace from that?

> #2  0x0000558b6223d46e in ExceptionalCondition (conditionName=conditionName@entry=0x558b623b2577 "tagPtr->blockNum !=
P_NEW",
>     errorType=errorType@entry=0x558b6229b016 "FailedAssertion",
>     fileName=fileName@entry=0x558b623b2598 "./build/../src/backend/storage/buffer/buf_table.c",
lineNumber=lineNumber@entry=125)
>     at ./build/../src/backend/utils/error/assert.c:67
> #3  0x0000558b620bafb9 in BufTableInsert (tagPtr=tagPtr@entry=0x7ffec8919330, hashcode=hashcode@entry=960067002,
buf_id=<optimizedout>) 
>     at ./build/../src/backend/storage/buffer/buf_table.c:125
> #4  0x0000558b620bf827 in BufferAlloc (foundPtr=0x7ffec891932b, strategy=0x0, blockNum=4294967295,
forkNum=MAIN_FORKNUM,
>     relpersistence=112 'p', smgr=0x558b62ed4b38) at ./build/../src/backend/storage/buffer/bufmgr.c:1234

Ah, thanks.  I don't think it's unreasonable for BufTableInsert to contain
that assertion --- we shouldn't be trying to allocate a buffer for an
illegal block number.

The regular error comes from mdextend, but that is too late under this
worldview, because smgrextend expects to be given a zero-filled buffer
to write out.  I think where we ought to be making the check is right
where ReadBuffer_common replaces P_NEW:

    /* Substitute proper block number if caller asked for P_NEW */
    if (isExtend)
+   {
        blockNum = smgrnblocks(smgr, forkNum);
+       if (blockNum == InvalidBlockNumber)
+            ereport(ERROR,
+                    (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                     errmsg("cannot extend file \"%s\" beyond %u blocks",
+                            relpath(smgr->smgr_rnode, forkNum),
+                            InvalidBlockNumber)));
+   }

Having done that, the check in md.c could be reduced to an Assert,
although there's something to be said for leaving it as-is as an
extra layer of defense.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Vik Fearing
Date:
Subject: Re: Non-decimal integer literals
Next
From: Masahiko Sawada
Date:
Subject: Re: Skipping logical replication transactions on subscriber side