pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
Multiple full page writes in a single checkpoint? 7+ messages / 2 participants [nested] [flat]
* Multiple full page writes in a single checkpoint? @ 2021-02-03 23:05 Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 0 siblings, 1 reply; 7+ messages in thread From: Bruce Momjian @ 2021-02-03 23:05 UTC (permalink / raw) To: pgsql-hackers Cluster file encryption plans to use the LSN and page number as the nonce for heap/index pages. I am looking into the use of a unique nonce during hint bit changes. (You need to use a new nonce for re-encrypting a page that changes.) log_hint_bits already gives us a unique nonce for the first hint bit change on a page during a checkpoint, but we only encrypt on page write to the file system, so I am researching if log_hint_bits will already generate a unique LSN for every page write to the file system, even if there are multiple hint-bit-caused page writes to the file system during a single checkpoint. (We already know this works for multiple checkpoints.) Our docs on full_page_writes states: When this parameter is on, the <productname>PostgreSQL</productname> server writes the entire content of each disk page to WAL during the first modification of that page after a checkpoint. and wal_log_hints states: When this parameter is <literal>on</literal>, the <productname>PostgreSQL</productname> server writes the entire content of each disk page to WAL during the first modification of that page after a checkpoint, even for non-critical modifications of so-called hint bits. However, imagine these steps: 1. checkpoint starts 2. page is modified by row or hint bit change 3. page gets a new LSN and is marked as dirty 4. page image is flushed to WAL 5. pages is written to disk and marked as clean 6. page is modified by data or hint bit change 7. pages gets a new LSN and is marked as dirty 8. page image is flushed to WAL 9. checkpoint completes 10. pages is written to disk and marked as clean Is the above case valid, and would it cause two full page writes to WAL? More specifically, wouldn't it cause every write of the page to the file system to use a new LSN? If so, this means wal_log_hints is sufficient to guarantee a new nonce for every page image, even for multiple hint bit changes and page writes during a single checkpoint, and there is then no need for a hit bit counter on the page --- the unique LSN does that for us. I know log_hint_bits was designed to fix torn pages, but it seems to also do exactly what cluster file encryption needs. If the above is all true, should we update the docs, READMEs, or C comments about this? I think the cluster file encryption patch would at least need to document that we need to keep this behavior, because I don't think log_hint_bits needs to behave this way for checksum purposes because of the way full page writes are processed during crash recovery. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> @ 2021-02-03 23:29 ` Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 0 siblings, 1 reply; 7+ messages in thread From: Andres Freund @ 2021-02-03 23:29 UTC (permalink / raw) To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-hackers Hi, On 2021-02-03 18:05:56 -0500, Bruce Momjian wrote: > log_hint_bits already gives us a unique nonce for the first hint bit > change on a page during a checkpoint, but we only encrypt on page write > to the file system, so I am researching if log_hint_bits will already > generate a unique LSN for every page write to the file system, even if > there are multiple hint-bit-caused page writes to the file system during > a single checkpoint. (We already know this works for multiple > checkpoints.) No, it won't: > However, imagine these steps: > > 1. checkpoint starts > 2. page is modified by row or hint bit change > 3. page gets a new LSN and is marked as dirty > 4. page image is flushed to WAL > 5. pages is written to disk and marked as clean > 6. page is modified by data or hint bit change > 7. pages gets a new LSN and is marked as dirty > 8. page image is flushed to WAL > 9. checkpoint completes > 10. pages is written to disk and marked as clean > > Is the above case valid, and would it cause two full page writes to WAL? > More specifically, wouldn't it cause every write of the page to the file > system to use a new LSN? No. 8) won't happen. Look e.g. at XLogSaveBufferForHint(): /* * Update RedoRecPtr so that we can make the right decision */ RedoRecPtr = GetRedoRecPtr(); /* * We assume page LSN is first data on *every* page that can be passed to * XLogInsert, whether it has the standard page layout or not. Since we're * only holding a share-lock on the page, we must take the buffer header * lock when we look at the LSN. */ lsn = BufferGetLSNAtomic(buffer); if (lsn <= RedoRecPtr) /* wal log hint bit */ The RedoRecPtr is determined at 1. and doesn't change between 4) and 8). The LSN for 4) has to be *past* the RedoRecPtr from 1). Therefore we don't do another FPW. Changing this is *completely* infeasible. In a lot of workloads it'd cause a *massive* explosion of WAL volume. Like quadratically. You'll need to find another way to generate a nonce. In the non-hint bit case you'll automatically have a higher LSN in 7/8 though. So you won't need to do anything about getting a higher nonce. For the hint bit case in 8 you could consider just using any LSN generated after 4 (preferrably already flushed to disk) - but that seems somewhat ugly from a debuggability POV :/. Alternatively you could just create tiny WAL record to get a new LSN, but that'll sometimes trigger new WAL flushes when the pages are dirtied. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> @ 2021-02-04 00:21 ` Bruce Momjian <bruce@momjian.us> 2021-02-04 01:00 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 0 siblings, 1 reply; 7+ messages in thread From: Bruce Momjian @ 2021-02-04 00:21 UTC (permalink / raw) To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers On Wed, Feb 3, 2021 at 03:29:13PM -0800, Andres Freund wrote: > > Is the above case valid, and would it cause two full page writes to WAL? > > More specifically, wouldn't it cause every write of the page to the file > > system to use a new LSN? > > No. 8) won't happen. Look e.g. at XLogSaveBufferForHint(): > > /* > * Update RedoRecPtr so that we can make the right decision > */ > RedoRecPtr = GetRedoRecPtr(); > > /* > * We assume page LSN is first data on *every* page that can be passed to > * XLogInsert, whether it has the standard page layout or not. Since we're > * only holding a share-lock on the page, we must take the buffer header > * lock when we look at the LSN. > */ > lsn = BufferGetLSNAtomic(buffer); > > if (lsn <= RedoRecPtr) > /* wal log hint bit */ > > The RedoRecPtr is determined at 1. and doesn't change between 4) and > 8). The LSN for 4) has to be *past* the RedoRecPtr from 1). Therefore we > don't do another FPW. OK, so, what is happening is that it knows the page LSN is after the start of the current checkpoint (the redo point), so it knows not do to a full page write again? Smart, and makes sense. > Changing this is *completely* infeasible. In a lot of workloads it'd > cause a *massive* explosion of WAL volume. Like quadratically. You'll > need to find another way to generate a nonce. Do we often do multiple writes to the file system of the same page during a single checkpoint, particularly only-hint-bit-modified pages? I didn't think so. > In the non-hint bit case you'll automatically have a higher LSN in 7/8 > though. So you won't need to do anything about getting a higher nonce. Yes, I was counting on that. :-) > For the hint bit case in 8 you could consider just using any LSN generated > after 4 (preferably already flushed to disk) - but that seems somewhat > ugly from a debuggability POV :/. Alternatively you could just create > tiny WAL record to get a new LSN, but that'll sometimes trigger new WAL > flushes when the pages are dirtied. Yes, that would make sense. I do need the first full page write during a checkpoint to be sure I don't have torn pages that have some part of the page encrypted with one LSN and a second part with a different LSN. You are right that I don't need a second full page write during the same checkpoint because a torn page would just restore the first full page write and throw away the second LSN and hint bit changes, which is fine. I hadn't gotten to ask about that until I found if the previous assumptions were true, which they were not. Is the logical approach here to modify XLogSaveBufferForHint() so if a page write is not needed, to create a dummy WAL record that just increments the WAL location and updates the page LSN? (Is there a small WAL record I should reuse?) I can try to add a hint-bit-page-write page counter, but that might overflow, and then we will need a way to change the LSN anyway. I am researching this so I can give a clear report on the impact of adding this feature. I will update the wiki once we figure this out. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> @ 2021-02-04 01:00 ` Andres Freund <andres@anarazel.de> 2021-02-04 01:07 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 0 siblings, 1 reply; 7+ messages in thread From: Andres Freund @ 2021-02-04 01:00 UTC (permalink / raw) To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-hackers Hi, On 2021-02-03 19:21:25 -0500, Bruce Momjian wrote: > On Wed, Feb 3, 2021 at 03:29:13PM -0800, Andres Freund wrote: > > Changing this is *completely* infeasible. In a lot of workloads it'd > > cause a *massive* explosion of WAL volume. Like quadratically. You'll > > need to find another way to generate a nonce. > > Do we often do multiple writes to the file system of the same page > during a single checkpoint, particularly only-hint-bit-modified pages? > I didn't think so. It can easily happen. Consider ringbuffer using scans (like vacuum, seqscan) - they'll force the buffer out to disk soon after it's been dirtied. And often will read the same page again a short bit later. Or just any workload that's a bit bigger than shared buffers (but data is in the OS cache). Subsequent scans will often have new hint bits to set. > Is the logical approach here to modify XLogSaveBufferForHint() so if a > page write is not needed, to create a dummy WAL record that just > increments the WAL location and updates the page LSN? > (Is there a small WAL record I should reuse?) I think an explicit record type would be better. Or a hint record without an associated FPW. > I can try to add a hint-bit-page-write page counter, but that might > overflow, and then we will need a way to change the LSN anyway. That's just a question of width... Greetings, Andres Freund ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-04 01:00 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> @ 2021-02-04 01:07 ` Bruce Momjian <bruce@momjian.us> 2021-02-04 03:28 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-04 18:49 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 0 siblings, 2 replies; 7+ messages in thread From: Bruce Momjian @ 2021-02-04 01:07 UTC (permalink / raw) To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers On Wed, Feb 3, 2021 at 05:00:19PM -0800, Andres Freund wrote: > Hi, > > On 2021-02-03 19:21:25 -0500, Bruce Momjian wrote: > > On Wed, Feb 3, 2021 at 03:29:13PM -0800, Andres Freund wrote: > > > Changing this is *completely* infeasible. In a lot of workloads it'd > > > cause a *massive* explosion of WAL volume. Like quadratically. You'll > > > need to find another way to generate a nonce. > > > > Do we often do multiple writes to the file system of the same page > > during a single checkpoint, particularly only-hint-bit-modified pages? > > I didn't think so. > > It can easily happen. Consider ringbuffer using scans (like vacuum, > seqscan) - they'll force the buffer out to disk soon after it's been > dirtied. And often will read the same page again a short bit later. Or > just any workload that's a bit bigger than shared buffers (but data is > in the OS cache). Subsequent scans will often have new hint bits to > set. Oh, good point. > > Is the logical approach here to modify XLogSaveBufferForHint() so if a > > page write is not needed, to create a dummy WAL record that just > > increments the WAL location and updates the page LSN? > > (Is there a small WAL record I should reuse?) > > I think an explicit record type would be better. Or a hint record > without an associated FPW. OK. > > I can try to add a hint-bit-page-write page counter, but that might > > overflow, and then we will need a way to change the LSN anyway. > > That's just a question of width... Yeah, the hint bit counter is just delaying the inevitasble, plus it changes the page format, which I am trying to avoid. Also, I need this dummy record only if the page is marked clean, meaning a write to the file system already happened in the current checkpoint --- should not be to bad. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-04 01:00 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 01:07 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> @ 2021-02-04 03:28 ` Bruce Momjian <bruce@momjian.us> 1 sibling, 0 replies; 7+ messages in thread From: Bruce Momjian @ 2021-02-04 03:28 UTC (permalink / raw) To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers On Wed, Feb 3, 2021 at 08:07:16PM -0500, Bruce Momjian wrote: > > > I can try to add a hint-bit-page-write page counter, but that might > > > overflow, and then we will need a way to change the LSN anyway. > > > > That's just a question of width... > > Yeah, the hint bit counter is just delaying the inevitable, plus it > changes the page format, which I am trying to avoid. Also, I need this > dummy record only if the page is marked clean, meaning a write > to the file system already happened in the current checkpoint --- should > not be to bad. Here is a proof-of-concept patch to do this. Thanks for your help. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee Attachments:^ permalink raw reply [nested|flat] 7+ messages in threadhint.diff (text/x-diff, 5.9K) download
From 11cb0ca7405a74105602fd63766e88c1fea8ea30 Mon Sep 17 00:00:00 2001 From: Bruce Momjian <bruce@momjian.us> Date: Wed, 3 Feb 2021 22:25:39 -0500 Subject: [PATCH] hint squash commit --- src/backend/access/rmgrdesc/xlogdesc.c | 6 +++++- src/backend/access/transam/xlog.c | 4 ++++ src/backend/access/transam/xloginsert.c | 26 ++++++++++++++++++++++++ src/backend/replication/logical/decode.c | 1 + src/backend/storage/buffer/bufmgr.c | 18 ++++++++++++++++ src/include/access/xloginsert.h | 2 ++ src/include/catalog/pg_control.h | 1 + 7 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c index 92cc7ea073..16a967bb71 100644 --- a/src/backend/access/rmgrdesc/xlogdesc.c +++ b/src/backend/access/rmgrdesc/xlogdesc.c @@ -80,7 +80,8 @@ xlog_desc(StringInfo buf, XLogReaderState *record) appendStringInfoString(buf, xlrec->rp_name); } - else if (info == XLOG_FPI || info == XLOG_FPI_FOR_HINT) + else if (info == XLOG_FPI || info == XLOG_FPI_FOR_HINT || + info == XLOG_HINT_LSN) { /* no further information to print */ } @@ -185,6 +186,9 @@ xlog_identify(uint8 info) case XLOG_FPI_FOR_HINT: id = "FPI_FOR_HINT"; break; + case XLOG_HINT_LSN: + id = "HINT_LSN"; + break; } return id; diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f03bd473e2..f67316ee07 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10260,6 +10260,10 @@ xlog_redo(XLogReaderState *record) UnlockReleaseBuffer(buffer); } } + else if (info == XLOG_HINT_LSN) + { + /* nothing to do here */ + } else if (info == XLOG_BACKUP_END) { XLogRecPtr startpoint; diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 7052dc245e..eebca0fc16 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -980,6 +980,32 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) return recptr; } +/* + * On the first hint bit change during a checkpoint, XLogSaveBufferForHint() + * writes a full page image to the WAL and returns a new LSN which can be + * assigned to the page. Cluster file encryption needs a new LSN for + * every hint bit page write because the LSN is used in the nonce, and + * the nonce must be unique. Therefore, this routine just advances the LSN + * so the page can be assigned a new LSN. + * + * Callable while holding just share lock on the buffer content. + * + * The LSN of the inserted wal record is returned if we had to write, + * InvalidXLogRecPtr otherwise. + * + * It is possible that multiple concurrent backends could attempt to write WAL + * records. In that case, multiple copies of the same block would be recorded + * in separate WAL records by different backends, though that is still OK from + * a correctness perspective. + */ +XLogRecPtr +XLogLSNForHint(void) +{ + XLogBeginInsert(); + + return XLogInsert(RM_XLOG_ID, XLOG_HINT_LSN); +} + /* * Write a WAL record containing a full image of a page. Caller is responsible * for writing the page to disk after calling this routine. diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index afa1df00d0..2ada89c6b1 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -223,6 +223,7 @@ DecodeXLogOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) case XLOG_FPW_CHANGE: case XLOG_FPI_FOR_HINT: case XLOG_FPI: + case XLOG_HINT_LSN: break; default: elog(ERROR, "unexpected RM_XLOG_ID record type: %u", info); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 561c212092..fd59f9fc1d 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3865,6 +3865,24 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) { dirtied = true; /* Means "will be dirtied by this action" */ + /* + * If the buffer is clean, and lsn is valid, it must be the + * first hint bit change of the checkpoint (the old page lsn + * is earlier than the RedoRecPtr). If the page is clean and + * the lsn is invalid, the page must have been already written + * during this checkpoint, and this is a new hint bit change. + * Such page changes usually don't create new page lsns, but we + * need one for cluster file encryption because the lsn is used as + * part of the nonce, and we don't want to reencrypt the page + * with the hint bit changes using the same nonce as the previous + * writes during this checkpoint. (It would expose the hint bit + * change locations.) Therefore we create a simple WAL record + * to advance the lsn, which can can then be assigned to the + * page. + */ + if (XLogRecPtrIsInvalid(lsn) /* XXX && file_encryption_method != 0 */) + lsn = XLogLSNForHint(); + /* * Set the page LSN if we wrote a backup block. We aren't supposed * to set this when only holding a share lock but as long as we diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index f1d8c39edf..a066f65229 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -61,6 +61,8 @@ extern void log_newpage_range(Relation rel, ForkNumber forkNum, BlockNumber startblk, BlockNumber endblk, bool page_std); extern XLogRecPtr XLogSaveBufferForHint(Buffer buffer, bool buffer_std); +extern XLogRecPtr XLogLSNForHint(void); + extern void InitXLogInsert(void); #endif /* XLOGINSERT_H */ diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h index e3f48158ce..36ac7dfbb8 100644 --- a/src/include/catalog/pg_control.h +++ b/src/include/catalog/pg_control.h @@ -76,6 +76,7 @@ typedef struct CheckPoint #define XLOG_END_OF_RECOVERY 0x90 #define XLOG_FPI_FOR_HINT 0xA0 #define XLOG_FPI 0xB0 +#define XLOG_HINT_LSN 0xC0 /* -- 2.20.1
* Re: Multiple full page writes in a single checkpoint? 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-04 01:00 ` Re: Multiple full page writes in a single checkpoint? Andres Freund <andres@anarazel.de> 2021-02-04 01:07 ` Re: Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> @ 2021-02-04 18:49 ` Bruce Momjian <bruce@momjian.us> 1 sibling, 0 replies; 7+ messages in thread From: Bruce Momjian @ 2021-02-04 18:49 UTC (permalink / raw) To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers; Masahiko Sawada <sawada.mshk@gmail.com> On Wed, Feb 3, 2021 at 08:07:16PM -0500, Bruce Momjian wrote: > > > I can try to add a hint-bit-page-write page counter, but that might > > > overflow, and then we will need a way to change the LSN anyway. > > > > That's just a question of width... > > Yeah, the hint bit counter is just delaying the inevitable, plus it > changes the page format, which I am trying to avoid. Also, I need this > dummy record only if the page is marked clean, meaning a write > to the file system already happened in the current checkpoint --- should > not be to bad. In looking your comments on Sawada-san's POC patch for buffer encryption: https://www.postgresql.org/message-id/20210112193431.2edcz776qjen7kao%40alap3.anarazel.de I see that he put a similar function call in exactly the same place I did, but you pointed out that he was inserting into WAL while holding a buffer lock. I restructured my patch to not make that same mistake, and modified it for non-permanent buffers --- attached. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com The usefulness of a cup is in its emptiness, Bruce Lee Attachments:^ permalink raw reply [nested|flat] 7+ messages in threadhint.diff (text/x-diff, 9.5K) download
From d535b08d264281348ab9e8eac645a5382ec35a8a Mon Sep 17 00:00:00 2001 From: Bruce Momjian <bruce@momjian.us> Date: Thu, 4 Feb 2021 13:43:56 -0500 Subject: [PATCH] hint squash commit --- src/backend/access/rmgrdesc/xlogdesc.c | 6 +- src/backend/access/transam/xlog.c | 4 ++ src/backend/access/transam/xloginsert.c | 16 +++++ src/backend/replication/logical/decode.c | 1 + src/backend/storage/buffer/bufmgr.c | 89 ++++++++++++++++-------- src/include/access/xloginsert.h | 2 + src/include/catalog/pg_control.h | 1 + 7 files changed, 90 insertions(+), 29 deletions(-) diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c index 92cc7ea073..16a967bb71 100644 --- a/src/backend/access/rmgrdesc/xlogdesc.c +++ b/src/backend/access/rmgrdesc/xlogdesc.c @@ -80,7 +80,8 @@ xlog_desc(StringInfo buf, XLogReaderState *record) appendStringInfoString(buf, xlrec->rp_name); } - else if (info == XLOG_FPI || info == XLOG_FPI_FOR_HINT) + else if (info == XLOG_FPI || info == XLOG_FPI_FOR_HINT || + info == XLOG_HINT_LSN) { /* no further information to print */ } @@ -185,6 +186,9 @@ xlog_identify(uint8 info) case XLOG_FPI_FOR_HINT: id = "FPI_FOR_HINT"; break; + case XLOG_HINT_LSN: + id = "HINT_LSN"; + break; } return id; diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f03bd473e2..f67316ee07 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10260,6 +10260,10 @@ xlog_redo(XLogReaderState *record) UnlockReleaseBuffer(buffer); } } + else if (info == XLOG_HINT_LSN) + { + /* nothing to do here */ + } else if (info == XLOG_BACKUP_END) { XLogRecPtr startpoint; diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 7052dc245e..7635a3d44d 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -980,6 +980,22 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) return recptr; } +/* + * On the first hint bit change during a checkpoint, XLogSaveBufferForHint() + * writes a full page image to the WAL and returns a new LSN which can be + * assigned to the page. Cluster file encryption needs a new LSN for + * every hint bit page write because the LSN is used in the nonce, and + * the nonce must be unique. Therefore, this routine just advances the LSN + * so the page can be assigned a new LSN. + */ +XLogRecPtr +XLogLSNForHint(void) +{ + XLogBeginInsert(); + + return XLogInsert(RM_XLOG_ID, XLOG_HINT_LSN); +} + /* * Write a WAL record containing a full image of a page. Caller is responsible * for writing the page to disk after calling this routine. diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c index afa1df00d0..2ada89c6b1 100644 --- a/src/backend/replication/logical/decode.c +++ b/src/backend/replication/logical/decode.c @@ -223,6 +223,7 @@ DecodeXLogOp(LogicalDecodingContext *ctx, XLogRecordBuffer *buf) case XLOG_FPW_CHANGE: case XLOG_FPI_FOR_HINT: case XLOG_FPI: + case XLOG_HINT_LSN: break; default: elog(ERROR, "unexpected RM_XLOG_ID record type: %u", info); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 561c212092..b9f3f76798 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3810,13 +3810,14 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) * If we need to protect hint bit updates from torn writes, WAL-log a * full page image of the page. This full page image is only necessary * if the hint bit update is the first change to the page since the - * last checkpoint. + * last checkpoint. If cluster file encryption is enabled, we also + * need to generate new page LSNs for non-first-page writes during + * a checkpoint. * * We don't check full_page_writes here because that logic is included * when we call XLogInsert() since the value changes dynamically. */ - if (XLogHintBitIsNeeded() && - (pg_atomic_read_u32(&bufHdr->state) & BM_PERMANENT)) + if (XLogHintBitIsNeeded()) { /* * If we must not write WAL, due to a relfilenode-specific @@ -3826,35 +3827,67 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) * * See src/backend/storage/page/README for longer discussion. */ - if (RecoveryInProgress() || - RelFileNodeSkippingWAL(bufHdr->tag.rnode)) + if (((pg_atomic_read_u32(&bufHdr->state) & BM_PERMANENT) + /* XXX || file_encryption_method != 0 */) && + (RecoveryInProgress() || + RelFileNodeSkippingWAL(bufHdr->tag.rnode))) return; + if (pg_atomic_read_u32(&bufHdr->state) & BM_PERMANENT) + { + /* + * If the block is already dirty because we either made a change + * or set a hint already, then we don't need to write a full page + * image. Note that aggressive cleaning of blocks dirtied by hint + * bit setting would increase the call rate. Bulk setting of hint + * bits would reduce the call rate... + * + * We must issue the WAL record before we mark the buffer dirty. + * Otherwise we might write the page before we write the WAL. That + * causes a race condition, since a checkpoint might occur between + * writing the WAL record and marking the buffer dirty. We solve + * that with a kluge, but one that is already in use during + * transaction commit to prevent race conditions. Basically, we + * simply prevent the checkpoint WAL record from being written + * until we have marked the buffer dirty. We don't start the + * checkpoint flush until we have marked dirty, so our checkpoint + * must flush the change to disk successfully or the checkpoint + * never gets written, so crash recovery will fix. + * + * It's possible we may enter here without an xid, so it is + * essential that CreateCheckpoint waits for virtual transactions + * rather than full transactionids. + */ + MyProc->delayChkpt = delayChkpt = true; + lsn = XLogSaveBufferForHint(buffer, buffer_std); + } + /* - * If the block is already dirty because we either made a change - * or set a hint already, then we don't need to write a full page - * image. Note that aggressive cleaning of blocks dirtied by hint - * bit setting would increase the call rate. Bulk setting of hint - * bits would reduce the call rate... - * - * We must issue the WAL record before we mark the buffer dirty. - * Otherwise we might write the page before we write the WAL. That - * causes a race condition, since a checkpoint might occur between - * writing the WAL record and marking the buffer dirty. We solve - * that with a kluge, but one that is already in use during - * transaction commit to prevent race conditions. Basically, we - * simply prevent the checkpoint WAL record from being written - * until we have marked the buffer dirty. We don't start the - * checkpoint flush until we have marked dirty, so our checkpoint - * must flush the change to disk successfully or the checkpoint - * never gets written, so crash recovery will fix. - * - * It's possible we may enter here without an xid, so it is - * essential that CreateCheckpoint waits for virtual transactions - * rather than full transactionids. + * For cluster file encryption, we generate a new page LSN + * for hint-bit non-permanent and non-first page writes. */ - MyProc->delayChkpt = delayChkpt = true; - lsn = XLogSaveBufferForHint(buffer, buffer_std); + if (/* XXX file_encryption_method != 0 && */ + XLogRecPtrIsInvalid(lsn) && + /* XXX can we check BM_DIRTY without a page lock? */ + !(pg_atomic_read_u32(&bufHdr->state) & BM_DIRTY)) + { + /* + * If the buffer is clean, and lsn is valid, it must be the + * first hint bit change of the checkpoint (the old page lsn + * is earlier than the RedoRecPtr). If the page is clean and + * the lsn is invalid, the page must have been already written + * during this checkpoint, and this is a new hint bit change. + * Such page changes usually don't create new page lsns, but we + * need one for cluster file encryption because the lsn is used as + * part of the nonce, and we don't want to reencrypt the page + * with the hint bit changes using the same nonce as the previous + * writes during this checkpoint. (It would expose the hint bit + * change locations.) Therefore we create a simple WAL record + * to advance the lsn, which can can then be assigned to the + * page below. + */ + lsn = XLogLSNForHint(); + } } buf_state = LockBufHdr(bufHdr); diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index f1d8c39edf..a066f65229 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -61,6 +61,8 @@ extern void log_newpage_range(Relation rel, ForkNumber forkNum, BlockNumber startblk, BlockNumber endblk, bool page_std); extern XLogRecPtr XLogSaveBufferForHint(Buffer buffer, bool buffer_std); +extern XLogRecPtr XLogLSNForHint(void); + extern void InitXLogInsert(void); #endif /* XLOGINSERT_H */ diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h index e3f48158ce..36ac7dfbb8 100644 --- a/src/include/catalog/pg_control.h +++ b/src/include/catalog/pg_control.h @@ -76,6 +76,7 @@ typedef struct CheckPoint #define XLOG_END_OF_RECOVERY 0x90 #define XLOG_FPI_FOR_HINT 0xA0 #define XLOG_FPI 0xB0 +#define XLOG_HINT_LSN 0xC0 /* -- 2.20.1
end of thread, other threads:[~2021-02-04 18:49 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-02-03 23:05 Multiple full page writes in a single checkpoint? Bruce Momjian <bruce@momjian.us> 2021-02-03 23:29 ` Andres Freund <andres@anarazel.de> 2021-02-04 00:21 ` Bruce Momjian <bruce@momjian.us> 2021-02-04 01:00 ` Andres Freund <andres@anarazel.de> 2021-02-04 01:07 ` Bruce Momjian <bruce@momjian.us> 2021-02-04 03:28 ` Bruce Momjian <bruce@momjian.us> 2021-02-04 18:49 ` Bruce Momjian <bruce@momjian.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox