Re: Reducing relcache memory usage 2: shrink sizeof(RelationData)

Hi Haibo,

Thanks for your review and kudos for providing test cases and fixes.
Much appreciated.

> I tested v3 independently on Linux x86_64, based on 9dbc53591a2. All
> eight attachments apply there without context changes. I concentrated
> on v3 0001-0006, following the cost/benefit concerns already raised by
> Matthias about 0007 and 0008.

I've removed 0007 and 0008 from the new patch set.

> I found a predicate-locking crash introduced by the union in 0004. The
> predicate code uses rd_index (and, with 0008, RelationGetIndex()) to
> decide whether a Relation is an index. For a rule-bearing table,
> rd_rules occupies the same union storage, so that test can interpret a
> RuleLock as index metadata.
> This reproduces with 0001-0006; 0008 is not required. A minimal case is:
>
> CREATE TABLE t (a int PRIMARY KEY);
> CREATE RULE r AS ON INSERT TO t DO ALSO SELECT 1;
> BEGIN ISOLATION LEVEL SERIALIZABLE;
> INSERT INTO t VALUES (1);
> TRUNCATE t;
> In a cassert build, TRUNCATE aborts at
> CheckTableForSerializableConflictIn(). At that point relkind is 'r',
> and rd_rules and the overlaid rd_index contain the same non-NULL address.
> The server process exits with SIGABRT. The unpatched baseline passes the
> same pg_regress test.
> The first attached patch adds RelationIsIndex(), based on relkind, and> uses it at the three predicate-locking classification sites. The second
> adds the pg_regress test. Both ordinary and partitioned indexes remain
> covered by the predicate.

Fixed the problem using your patch. I don't think we should include the
test cases as it's very specific. Opinions?

> I also reproduced an ERROR-path problem from 0006 using an injection
> point at its new RelationPartitionInfo allocation inside
> RelationRebuildRelation(). After an ALTER TABLE added a column to a
> partitioned table, the injected ERROR occurred after the descriptor swap.
> Rollback restored the two-column catalog state, but the same backend kept
> the three-column descriptor and then reported:
>
> could not find inherited attribute "key_added"

Good catch!

> An equivalent general swap window exists without this series. The v3
> regression is specifically that 0006 adds an allocation failure point to
> the region. I did not observe or claim a real OOM.
>
> The third patch allocates destination partition metadata before the swap
> and before reparenting a completed partition-key context. It also avoids
> reading table-side partition metadata for index relations. With the
> allocation moved before the swap, an ERROR leaves the old descriptor intact
> and marked invalid; the next lookup rebuilds it normally after rollback.
> Ten repeated subtransaction failures, partitioned indexes, and
> debug_discard_caches=1 also passed without an external invalidation.

Fixed the problem using your patch.

> Finally, the init-file loader reconstructs index access fields but does
> not reset rd_indexprs, rd_indpred, rd_exclops, rd_exclprocs, or
> rd_exclstrats. Those are serialized process pointers whose contents are
> supposed to be initialized lazily. The fourth patch resets the five
> fields without changing the init-file format. I did not find a reachable
> failure with current catalog indexes, so I regard this as an
> initialization correctness hazard rather than a demonstrated SQL crash.

Fixed as well. Additionally moved into the else branch of the check

if (!RelationIsIndex(rel))

so that all code that resets lazily built fields is in one place.

> On Linux, sizeof(RelationData) is 496 bytes at the baseline, 272 through
> 0006 and 0007, and 264 through 0008. Under the same 8 KiB Slab setup,
> 0006 and 0007 both used 302 blocks and 2,474,160 total bytes for a workload
> loading 8,731 relations. This supports the limited-benefit point already
> made in the review discussion.

With the latest patch set (excluding 0007 and 0008), we go from 496
bytes on master to 248 bytes.

> One additional issue: original v3 0008 does not build on this base because
> execReplication.c still has two rd_index references. I did not repair it
> silently or use that configuration for runtime results.

Removed in latest patch set.

Attached is v4 of the patch set.

Beyond that:

1. Bumped RELCACHE_INIT_FILEMAGIC in any patch that makes structural
changes to struct RelationData, so that PostgreSQL rebuilds old init
files if necessary.

2. Moved many more members that are relation-only to the corresponding
union leg.

3. Moved members related to partitioning out of the relation-only table
leg because indexes can be partitioned as well.

--
David Geier