Re: SLRUs in the main buffer pool, redux

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SLRUs in the main buffer pool, redux
Date: 2022-07-25 06:54:25
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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

Attachment Content-Type Size
v3-0001-WIP-Move-SLRU-data-into-the-regular-buffer-pool.patch text/x-patch 171.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2022-07-25 07:28:36 Re: predefined role(s) for VACUUM and ANALYZE
Previous Message Bharath Rupireddy 2022-07-25 06:49:40 Re: Add last failed connection error message to pg_stat_wal_receiver