Re: Separate memory contexts for relcache and catcache

Hi,

On 2023-08-09 15:02:31 +0300, Melih Mutlu wrote:
> To quickly show how pg_backend_memory_contexts would look like, I did the
> following:
> 
> -Create some tables:
> SELECT 'BEGIN;' UNION ALL SELECT format('CREATE TABLE %1$s(id serial
> primary key, data text not null unique)', 'test_'||g.i) FROM
> generate_series(0, 1000) g(i) UNION ALL SELECT 'COMMIT;';\gexec
> 
> -Open a new connection and query pg_backend_memory_contexts [1]:
> This is what you'll see before and after the patch.
> -- HEAD:
>         name        | used_bytes | free_bytes | total_bytes
> --------------------+------------+------------+-------------
>  CacheMemoryContext |     467656 |      56632 |      524288
>  index info         |     111760 |      46960 |      158720
>  relation rules     |       4416 |       3776 |        8192
> (3 rows)
> 
> -- Patch:
>          name          | used_bytes | free_bytes | total_bytes
> -----------------------+------------+------------+-------------
>  CatCacheMemoryContext |     217696 |      44448 |      262144
>  RelCacheMemoryContext |     248264 |      13880 |      262144
>  index info            |     111760 |      46960 |      158720
>  CacheMemoryContext    |       2336 |       5856 |        8192
>  relation rules        |       4416 |       3776 |        8192
> (5 rows)

Have you checked what the source of the remaining allocations in
CacheMemoryContext are?


One thing that I had observed previously and reproduced with this patch, is
that the first backend starting after a restart uses considerably more memory:

first:
┌───────────────────────┬────────────┬────────────┬─────────────┐
│         name          │ used_bytes │ free_bytes │ total_bytes │
├───────────────────────┼────────────┼────────────┼─────────────┤
│ CatCacheMemoryContext │     370112 │     154176 │      524288 │
│ RelCacheMemoryContext │     244136 │      18008 │      262144 │
│ index info            │     104392 │      45112 │      149504 │
│ CacheMemoryContext    │       2304 │       5888 │        8192 │
│ relation rules        │       3856 │        240 │        4096 │
└───────────────────────┴────────────┴────────────┴─────────────┘

second:
┌───────────────────────┬────────────┬────────────┬─────────────┐
│         name          │ used_bytes │ free_bytes │ total_bytes │
├───────────────────────┼────────────┼────────────┼─────────────┤
│ CatCacheMemoryContext │     215072 │      47072 │      262144 │
│ RelCacheMemoryContext │     243856 │      18288 │      262144 │
│ index info            │     104944 │      47632 │      152576 │
│ CacheMemoryContext    │       2304 │       5888 │        8192 │
│ relation rules        │       3856 │        240 │        4096 │
└───────────────────────┴────────────┴────────────┴─────────────┘

This isn't caused by this patch, but it does make it easier to pinpoint than
before.  The reason is fairly simple: On the first start we start without
being able to use relcache init files, in later starts we can. The reason the
size increase is in CatCacheMemoryContext, rather than RelCacheMemoryContext,
is simple: When using the init file the catcache isn't used, when not, we have
to query the catcache a lot to build the initial relcache contents.


Given the size of both CatCacheMemoryContext and RelCacheMemoryContext in a
new backend, I think it might be worth using non-default aset parameters. A
bit ridiculous to increase block sizes from 8k upwards in every single
connection made to postgres ever.


> - Run select on all tables
> SELECT format('SELECT count(*) FROM %1$s', 'test_'||g.i) FROM
> generate_series(0, 1000) g(i);\gexec
> 
> - Then check pg_backend_memory_contexts [1] again:
> --HEAD
>         name        | used_bytes | free_bytes | total_bytes
> --------------------+------------+------------+-------------
>  CacheMemoryContext |    8197344 |     257056 |     8454400
>  index info         |    2102160 |     113776 |     2215936
>  relation rules     |       4416 |       3776 |        8192
> (3 rows)
> 
> --Patch
>          name          | used_bytes | free_bytes | total_bytes
> -----------------------+------------+------------+-------------
>  RelCacheMemoryContext |    4706464 |    3682144 |     8388608
>  CatCacheMemoryContext |    3489384 |     770712 |     4260096
>  index info            |    2102160 |     113776 |     2215936
>  CacheMemoryContext    |       2336 |       5856 |        8192
>  relation rules        |       4416 |       3776 |        8192
> (5 rows)
> 
> You can see that CacheMemoryContext does not use much memory without
> catcache and relcache (at least in cases similar to above), and it's easy
> to bloat catcache and relcache. That's why I think it would be useful to
> see their usage separately.

Yes, I think it'd be quite useful. There's ways to bloat particularly catcache
much further, and it's hard to differentiate that from other sources of bloat
right now.


> +static void
> +CreateCatCacheMemoryContext()

We typically use (void) to differentiate from an older way of function
declarations that didn't have argument types.


> +{
> +    if (!CacheMemoryContext)
> +        CreateCacheMemoryContext();

I wish we just made sure that cache memory context were created in the right
place, instead of spreading this check everywhere...


> @@ -3995,9 +3998,9 @@ RelationCacheInitializePhase2(void)
>          return;
>  
>      /*
> -     * switch to cache memory context
> +     * switch to relcache memory context
>       */
> -    oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
> +    oldcxt = MemoryContextSwitchTo(RelCacheMemoryContext);
>  
>      /*
>       * Try to load the shared relcache cache file.  If unsuccessful, bootstrap
> @@ -4050,9 +4053,9 @@ RelationCacheInitializePhase3(void)
>      RelationMapInitializePhase3();
>  
>      /*
> -     * switch to cache memory context
> +     * switch to relcache memory context
>       */
> -    oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
> +    oldcxt = MemoryContextSwitchTo(RelCacheMemoryContext);
>  
>      /*
>       * Try to load the local relcache cache file.  If unsuccessful, bootstrap

I'd just delete these comments, they're just pointlessly restating the code.


Greetings,

Andres Freund