Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS - Mailing list pgsql-hackers

From Craig Ringer
Subject Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS
Date
Msg-id CAMsr+YHxLTw3FEd8VhPT9ScD_deKKushm7wwF8YsXSeq3YWK+A@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS
Re: [PATCH] Runtime control of CLOBBER_CACHE_ALWAYS
List pgsql-hackers

On Tue, 27 Oct 2020 at 16:34, Peter Eisentraut <peter.eisentraut@2ndquadrant.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
Attachment

pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: should INSERT SELECT use a BulkInsertState?
Next
From: Craig Ringer
Date:
Subject: Re: Two fsync related performance issues?