Thread: Assertion failure with small block sizes
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
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
"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
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
"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
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