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 |
Message-ID: | ececc014-cf3d-acbb-35fd-e99b5c8dfb4e@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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 |
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 |