Followup: Syscaches should store negative entries, too - Mailing list pgsql-hackers

From Tom Lane
Subject Followup: Syscaches should store negative entries, too
Date
Msg-id 5844.1015183933@sss.pgh.pa.us
Whole thread Raw
List pgsql-hackers
I've now committed changes to make the system caches store negative as
well as positive entries, per the discussion from about a month ago:
http://archives.postgresql.org/pgsql-hackers/2002-01/msg01314.php

I have not done very much performance testing, but preliminary results
say that the change had the expected effect.  On a 100-query pgbench
run, I had these results a month ago:
Catcache totals: 43 tup, 14519 srch, 13976 hits, 43 loads, 500 not found
while with CVS tip I now get:
Catcache totals: 48 tup, 15019 srch, 14476+495=14971 hits, 43+5=48 loads, 0 invals, 0 discards
(Hits and new-entry loads are both expressed in the form pos+neg=total.)
This confirms my guess that the failing searches were all looking for
the same five tuples.  The net number of catalog indexscans has been
reduced from 543 to 48.  The improvement is much less spectacular on the
regression tests, which don't have such a repetitive query structure;
but several of the regression tests show factor-of-2 reductions in
catalog probes.

inval.c has gotten a lot simpler since it no longer needs to distinguish
insert and delete operations.  It now just keeps two lists: one of inval
events caused in the current command (and not yet reflected to the
cache) and one of events in prior commands of the current transaction
(which have been reflected to this backend's cache, but not yet reported
to other backends).  At CommandCounterIncrement time we process the
current-command list against the local caches and then nconc it onto the
other list.  The total time and memory costs of storing the inval lists
should actually be less than before, since no event needs to be stored
in more than one list.

I did observe some interesting breakage: creation of tables that inherit
constraints or column defaults from a parent table failed with the new
code.  On investigation, it turned out that StoreDefaults() was doing
CommandCounterIncrement at a time when we'd created pg_class and
pg_attribute rows claiming the new table had constraints or defaults,
but we hadn't yet made any pg_relcheck or pg_attrdef entries for them.
In the new code, the CommandCounterIncrement causes a rebuild of the
new table's relcache entry, and various sanity checks were failing.
The fix was to treat inherited constraints/defaults more like the normal
case: we first create pg_class and pg_attribute rows without any mention
of constraints or defaults, and then update those rows when we've stored
the auxiliary info.

It's possible that there are similar bugs lurking elsewhere.  I did some
casual exercising of ALTER TABLE but didn't turn up any problems.
Anyway one should be wary of doing CommandCounterIncrement partway
through a set of changes to a relation.

One other change is that inval.c now exposes a routine that can be used
to queue a relcache flush event without necessarily having modified the
relation's pg_class row.  I changed heap.c and index.c to use this in
place of doing no-op pg_class updates, but there may be some other
places that could be improved as well.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Neil Conway
Date:
Subject: Re: new hashing function
Next
From: "Jim Buttafuoco"
Date:
Subject: Re: Status of index location patch