Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS

On Tue, 27 Oct 2020 at 16:34, Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:

> On 2020-09-25 09:40, Craig Ringer wrote:
> > While working on extensions I've often wanted to enable cache clobbering
> > for a targeted piece of code, without paying the price of running it for
> > all backends during postgres startup and any initial setup tasks that
> > are required.
> >
> > So here's a patch that, when CLOBBER_CACHE_ALWAYS or
> > CLOBBER_CACHE_RECURSIVE are defined, adds a GUC named
> > clobber_cache_depth . It defaults to 1 for CLOBBER_CACHE_ALWAYS or 3 for
> > CLOBBER_CACHE_RECURSIVE to match the existing compiled-in behaviour. But
> > with this change it's now possible to run Pg with clobber_cache_depth=0
> > then set it to 1 only for targeted tests.
>
> I think it would be great if the cache clobber code is always compiled
> in (but turned off) when building with assertions. The would reduce the
> number of hoops to jump through to actually use this locally and reduce
> the number of surprises from the build farm.
>

I implemented something a bit like that for a patched postgres where it
became an additional configure option. It's easy enough to enable it
automatically for USING_CASSERT instead, and I think that's sensible, so
I've adapted the patch to do so.

As initially written the patch defined the clobber control GUC only if
cache clobber control is compiled in. But we don't do that for anything
else, so I've changed it to always define the GUC, which will be ignored
without effect if no cache clobber control is compiled in. I also renamed
the GUC to debug_clobber_cache_depth to match other debug GUCs.

For backward compatibility, you can arrange it so that the built-in
> default of clobber_cache_depth is 1 or 3 if CLOBBER_CACHE_ALWAYS or
> CLOBBER_CACHE_RECURSIVE are defined.
>

It already does that - see diff hunk for src/include/utils/inval.h in prior
patch.

I've just changed it to default to 0 if neither are defined and moved it to
pg_config_manual.h .

I noticed that I missed handling of RECOVER_RELATION_BUILD_MEMORY in the
earlier patch - the relcache was eagerly freeing relation descriptor memory
without regard for the current clobber_cache_depth value. I've changed that
in the updated patch so that RelationBuildDesc only frees memory eagerly if
clobber_cache_depth is actually > 0 or if RECOVER_RELATION_BUILD_MEMORY has
been explicitly defined to 1. It also preserves the original behaviour of
not eagerly freeing memory even when cache clobber is enabled if
RECOVER_RELATION_BUILD_MEMORY is explicitly defined to 0.

Both CLOBBER_CACHE_ENABLED and RECOVER_RELATION_BUILD_MEMORY are now shown
in pg_config_manual.h .

A small entry has been added to the docs too, under developer options.

The changes add a little more noise than I'd prefer, but I didn't want to
vanish support for the compile-time constants or introduce surprising
behaviour.

To try it out, apply the patch (git am), build with --enable-cassert, then
compare:

make -C src/test/regress check

and

PGOPTIONS="-c debug_clobber_cache_depth=1" \
make -C src/test/regress check

The speed difference will be obvious if nothing else!

It's much more practical using make check-tests and a specific TESTS= list.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise