Thread: Assertion failure with small block sizes

Assertion failure with small block sizes

From
Gregory Stark
Date:
Testing Postgres with a small block size runs into an assertion failure when
it tries to toast a pg_proc tuple during initdb. I think the assertion is just
wrong and RELKIND_UNCATALOGUED is valid here. Other places in the code check
for both before, for example, creating toast tables.

creating template1 database in /home/stark/src/local-HEAD/pgsql/src/test/regress/./tmp_check/data/base/1 ... TRAP:
FailedAssertion("!(rel->rd_rel->relkind== 'r')", File: "tuptoaster.c", Line: 440) 


--- tuptoaster.c    13 Oct 2007 22:34:06 +0100    1.78
+++ tuptoaster.c    14 Oct 2007 15:37:17 +0100
@@ -437,7 +437,8 @@
      * We should only ever be called for tuples of plain relations ---
      * recursing on a toast rel is bad news.
      */
-    Assert(rel->rd_rel->relkind == RELKIND_RELATION);
+    Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
+           rel->rd_rel->relkind == RELKIND_UNCATALOGED);

     /*
      * Get the tuple descriptor and break down the tuple(s) into fields.


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Re: Assertion failure with small block sizes

From
Tom Lane
Date:
Gregory Stark <stark@enterprisedb.com> writes:
> Testing Postgres with a small block size runs into an assertion failure when
> it tries to toast a pg_proc tuple during initdb. I think the assertion is just
> wrong and RELKIND_UNCATALOGUED is valid here.

Uh, what makes you think the assertion is the only problem?  The toast
table won't exist yet.

How small is "small" anyway?

            regards, tom lane

Re: Assertion failure with small block sizes

From
Gregory Stark
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> Gregory Stark <stark@enterprisedb.com> writes:
>> Testing Postgres with a small block size runs into an assertion failure when
>> it tries to toast a pg_proc tuple during initdb. I think the assertion is just
>> wrong and RELKIND_UNCATALOGUED is valid here.
>
> Uh, what makes you think the assertion is the only problem?  The toast
> table won't exist yet.

Well tuptoaster doesn't try to store anything external if there's no toast
table anyways. With the assertion modified it passes the regression tests
(modulo row orderings).

> How small is "small" anyway?

This was 1k with 256 toast target/threshold.

If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the same
line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the
assertion then it passes all regression tests even if I push
TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as
possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as
well.


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Re: Assertion failure with small block sizes

From
Tom Lane
Date:
Gregory Stark <stark@enterprisedb.com> writes:
> If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the same
> line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the
> assertion then it passes all regression tests even if I push
> TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as
> possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as
> well.

Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
heapam.c, so that it explicitly tries to toast only in plain tables,
rather than adding more exclusion cases.  Thoughts?

            regards, tom lane

Re: Assertion failure with small block sizes

From
Gregory Stark
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> Gregory Stark <stark@enterprisedb.com> writes:
>> If I push the TOAST_TUPLES_PER_PAGE up to 16 I get another failure on the same
>> line from trying to toast a sequence. If I add RELKIND_SEQUENCE to the
>> assertion then it passes all regression tests even if I push
>> TOAST_TUPLES_PER_PAGE up to 1024 -- ie, try to toast everything as far as
>> possible. Perhaps heapam.c:1761 should just check for RELKIND_SEQUENCE as
>> well.
>
> Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
> heapam.c, so that it explicitly tries to toast only in plain tables,
> rather than adding more exclusion cases.  Thoughts?

Well RELKIND_UNCATALOGED can be usefully toasted in that data can be
compressed internally. I suppose we know none normally receive such treatment
at normal block sizes. If we want to make the tuple threshold configurable
down the line would we want it affecting initdb bootstrapping? My experiments
show it isn't a problem but I don't particularly see any reason why it's
advantageous.

We may want some future relkinds to be toastable but I don't see a problem
adding new cases to the test.


--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Re: Assertion failure with small block sizes

From
Tom Lane
Date:
Gregory Stark <stark@enterprisedb.com> writes:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
>> Hmm.  I'm inclined to reverse the tests (there are 3 not just 1) in
>> heapam.c, so that it explicitly tries to toast only in plain tables,
>> rather than adding more exclusion cases.  Thoughts?

> Well RELKIND_UNCATALOGED can be usefully toasted in that data can be
> compressed internally.

But by the time we are inserting any data that needs compression, the
relkind should not be that anymore.  This would only be an issue if
pg_proc.h itself contained DATA() lines long enough to need toasting.
I argue that that isn't true and isn't likely to become true.  (See
ts_debug() for an example of deliberately avoiding such a case...)

            regards, tom lane