Re: SLRUs in the main buffer pool, redux

On 21/07/2022 16:23, Yura Sokolov wrote:
> Good day, Thomas
> 
> В Пт, 27/05/2022 в 23:24 +1200, Thomas Munro пишет:
>> Rebased, debugged and fleshed out a tiny bit more, but still with
>> plenty of TODO notes and questions.  I will talk about this idea at
>> PGCon, so I figured it'd help to have a patch that actually applies,
>> even if it doesn't work quite right yet.  It's quite a large patch but
>> that's partly because it removes a lot of lines...
> 
> Looks like it have to be rebased again.

Here's a rebase.

I'll write a separate post with my thoughts on the high-level design of 
this, but first a couple of more detailed issues:

In RecordTransactionCommit(), we enter a critical section, and then call 
TransactionIdCommitTree() to update the CLOG pages. That now involves a 
call to ReadBuffer_common(), which in turn calls 
ResourceOwnerEnlargeBuffers(). That can fail, because it might require 
allocating memory, which is forbidden in a critical section. I ran into 
an assertion about that with "make check" when I was playing around with 
a heavily modified version of this patch. Haven't seen it with your 
original one, but I believe that's just luck.

Calling ResourceOwnerEnlargeBuffers() before entering the critical 
section would probably fix that, although I'm a bit worried about having 
the Enlarge call so far away from the point where it's needed.

> +void
> +CheckPointSLRU(void)
> +{
> +       /* Ensure that directory entries for new files are on disk. */
> +       for (int i = 0; i < lengthof(defs); ++i)
> +       {
> +               if (defs[i].synchronize)
> +                       fsync_fname(defs[i].path, true);
> +       }
> +}
> +

Is it really necessary to fsync() the directories? We don't do that 
today for the SLRUs, and we don't do it for the tablespace/db 
directories holding relations.

- Heikki