public inbox for [email protected]
help / color / mirror / Atom feedRe: Small run-time pruning doc fix
20+ messages / 5 participants
[nested] [flat]
* Re: Small run-time pruning doc fix
@ 2018-11-02 07:30 Peter Eisentraut <[email protected]>
2018-11-04 05:23 ` Re: Small run-time pruning doc fix David Rowley <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Peter Eisentraut @ 2018-11-02 07:30 UTC (permalink / raw)
To: David Rowley <[email protected]>; PostgreSQL Hackers <[email protected]>
On 08/10/2018 10:22, David Rowley wrote:
> Before 5220bb7533f a note in ddl.sgml used to mention that run-time
> pruning was only implemented for Append. When we got MergeAppend
> support the commit updated this to mention MergeAppend is supported
> too. This is slightly weird as it's not all that obvious what exactly
> isn't supported when we mention:
>
> <para>
> Both of these behaviors are likely to be changed in a future release
> of <productname>PostgreSQL</productname>.
> </para>
>
> The attached patch updates this to mention that ModifyTable is
> unsupported which I think makes the above fragment make sense again.
<para>
- Execution-time partition pruning currently occurs for the
+ Execution-time partition pruning currently only occurs for the
<literal>Append</literal> and <literal>MergeAppend</literal> node
types.
+ It is not yet implemented for the <literal>ModifyTable</literal> node
+ type.
</para>
Isn't this implied by the preceding paragraph
Currently, pruning of partitions during the planning of an UPDATE or
DELETE command is implemented using the constraint exclusion method
?
Also, could there be other node types that could benefit from partition
pruning that are not implemented yet? Not sure we want to mention all
of them.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Small run-time pruning doc fix
2018-11-02 07:30 Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
@ 2018-11-04 05:23 ` David Rowley <[email protected]>
2018-11-12 13:46 ` Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: David Rowley @ 2018-11-04 05:23 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Thanks for looking at this.
On 2 November 2018 at 20:30, Peter Eisentraut
<[email protected]> wrote:
> <para>
> - Execution-time partition pruning currently occurs for the
> + Execution-time partition pruning currently only occurs for the
> <literal>Append</literal> and <literal>MergeAppend</literal> node
> types.
> + It is not yet implemented for the <literal>ModifyTable</literal> node
> + type.
> </para>
>
> Isn't this implied by the preceding paragraph
>
> Currently, pruning of partitions during the planning of an UPDATE or
> DELETE command is implemented using the constraint exclusion method
That paragraph is talking about plan-time partition pruning. The
paragraph the patch changes is mentioning execution-time partition
pruning. In PG11 we have plan-time partition pruning for Append and
MergeAppend. UPDATE/DELETE does pruning, but it uses the constraint
exclusion code which does not work for HASH partitioned tables. In
PG11 we only have run-time partition pruning for Append. MergeAppend
support was only added in 5220bb7533 (PG12).
> Also, could there be other node types that could benefit from partition
> pruning that are not implemented yet? Not sure we want to mention all
> of them.
I'm unsure what might exist in the future, but there are currently
only 3 node types that take a list of subpaths. I think if we get the
support for ModifyTable later, then likely the paragraph edited in
this patch would disappear. In fact, the entire note would disappear
as adding run-time pruning for ModifyTable will require the planner to
use the new partition pruning code. There's a patch for that in [1],
so maybe it'll all be a little less confusing in PG12.
[1] https://commitfest.postgresql.org/20/1778/
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Small run-time pruning doc fix
2018-11-02 07:30 Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
2018-11-04 05:23 ` Re: Small run-time pruning doc fix David Rowley <[email protected]>
@ 2018-11-12 13:46 ` Peter Eisentraut <[email protected]>
2018-11-12 21:20 ` Re: Small run-time pruning doc fix David Rowley <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Peter Eisentraut @ 2018-11-12 13:46 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On 04/11/2018 06:23, David Rowley wrote:
> Thanks for looking at this.
>
> On 2 November 2018 at 20:30, Peter Eisentraut
> <[email protected]> wrote:
>> <para>
>> - Execution-time partition pruning currently occurs for the
>> + Execution-time partition pruning currently only occurs for the
>> <literal>Append</literal> and <literal>MergeAppend</literal> node
>> types.
>> + It is not yet implemented for the <literal>ModifyTable</literal> node
>> + type.
>> </para>
>>
>> Isn't this implied by the preceding paragraph
>>
>> Currently, pruning of partitions during the planning of an UPDATE or
>> DELETE command is implemented using the constraint exclusion method
>
> That paragraph is talking about plan-time partition pruning. The
> paragraph the patch changes is mentioning execution-time partition
> pruning. In PG11 we have plan-time partition pruning for Append and
> MergeAppend. UPDATE/DELETE does pruning, but it uses the constraint
> exclusion code which does not work for HASH partitioned tables. In
> PG11 we only have run-time partition pruning for Append. MergeAppend
> support was only added in 5220bb7533 (PG12).
I see now. The actual documentation change was separate in db72302b0a8.
Committed your change.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Small run-time pruning doc fix
2018-11-02 07:30 Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
2018-11-04 05:23 ` Re: Small run-time pruning doc fix David Rowley <[email protected]>
2018-11-12 13:46 ` Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
@ 2018-11-12 21:20 ` David Rowley <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: David Rowley @ 2018-11-12 21:20 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On 13 November 2018 at 02:46, Peter Eisentraut
<[email protected]> wrote:
> Committed your change.
Thanks.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Large Pages and Super Pages for PostgreSQL
@ 2022-01-16 07:32 Thomas Munro <[email protected]>
2022-11-07 22:59 ` Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Munro @ 2022-01-16 07:32 UTC (permalink / raw)
To: DEVOPS_WwIT <[email protected]>; +Cc: pgsql-hackers; ZHU XIAN WEN <[email protected]>
On Sun, Jan 16, 2022 at 6:03 PM DEVOPS_WwIT <[email protected]> wrote:
> Solaris and FreeBSD supports large/super pages, and can be used
> automatically by applications.
>
> Seems Postgres can't use the large/super pages on Solaris and FreeBSD
> os(I think can't use the large/super page HPUX and AIX), is there anyone
> could take a look?
Hello,
I can provide some clues and partial answers about page size on three
of the OSes you mentioned:
1. Solaris: I haven't used that OS for a long time, but I thought it
was supposed to promote memory to larger pages sizes transparently
with some heuristics. To control page size explicitly, it *looks*
like memcntl(2) with command MHA_MAPSIZE_VA could be used; that's what
the man page says, anyway. If someone is interested in writing a
patch to do that, I'd be happy to review it and test it on illumos...
2. AIX: We *nearly* made this work recently[1]. The summary is that
AIX doesn't have a way to control the page size of anonymous shared
mmap memory (our usual source of shared memory), so you have to use
SystemV shared memory if you want non-default page size for shared
memory. We got as far as adding the option shared_memory_type=sysv,
and the next step is pretty easy: just pass in some magic flags. This
just needs someone with access and motivation to pick up that work...
3. FreeBSD: FreeBSD does transparently migrate PostgreSQL memory to
"super" pages quite well in my experience, but there is also a new
facility in FreeBSD 13 to ask for specific page sizes explicitly. I
wrote a quick and dirty patch to enable PostgreSQL's huge_pages and
huge_page_size settings to work with that interface, but I haven't yet
got as far as testing it very hard or proposing it... but here it is,
if you like experimental code[2].
I don't know about HP-UX. I think it might be dead, Jim.
[1] https://www.postgresql.org/message-id/flat/HE1PR0202MB28126DB4E0B6621CC6A1A91286D90%40HE1PR0202MB281...
[2] https://github.com/macdice/postgres/commit/a71aafe5582c2e61005af0d16ca82eed89445a67
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Large Pages and Super Pages for PostgreSQL
2022-01-16 07:32 Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
@ 2022-11-07 22:59 ` Thomas Munro <[email protected]>
2022-11-30 07:29 ` Re: Large Pages and Super Pages for PostgreSQL ZHU XIAN WEN <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Munro @ 2022-11-07 22:59 UTC (permalink / raw)
To: DEVOPS_WwIT <[email protected]>; +Cc: pgsql-hackers; ZHU XIAN WEN <[email protected]>
On Sun, Jan 16, 2022 at 8:32 PM Thomas Munro <[email protected]> wrote:
> On Sun, Jan 16, 2022 at 6:03 PM DEVOPS_WwIT <[email protected]> wrote:
> > Solaris and FreeBSD supports large/super pages, and can be used
> > automatically by applications.
> >
> > Seems Postgres can't use the large/super pages on Solaris and FreeBSD
> > os(I think can't use the large/super page HPUX and AIX), is there anyone
> > could take a look?
>
> 3. FreeBSD: FreeBSD does transparently migrate PostgreSQL memory to
> "super" pages quite well in my experience, but there is also a new
> facility in FreeBSD 13 to ask for specific page sizes explicitly. I
> wrote a quick and dirty patch to enable PostgreSQL's huge_pages and
> huge_page_size settings to work with that interface, but I haven't yet
> got as far as testing it very hard or proposing it... but here it is,
> if you like experimental code[2].
I was reminded to rebase that and tidy it up a bit, by recent
discussion of page table magic in other threads. Documentation of
these interfaces is sparse to put it mildly (I may try to improve that
myself) but basically the terminology is "super" for pages subject to
promotion/demotion, and "large" when explicitly managed. Not
proposing for commit right now as I need to learn more about all this
and there are some policy decisions lurking in here (eg synchronous
defrag vs nowait depending on flags), but the patch may be useful for
experimentation. For example, it allows huge_page_size=1GB if your
system can handle that.
Attachments:
[text/x-patch] 0001-Support-huge_pages-and-huge_page_size-on-FreeBSD.patch (4.5K, ../../CA+hUKGLmBWHF6gusP55R7jVS1=6T=GphbZpUXiOgMMHDUkVCgw@mail.gmail.com/2-0001-Support-huge_pages-and-huge_page_size-on-FreeBSD.patch)
download | inline diff:
From b76dc0e5a472824aaa87de4f6c1f6db26c810c7f Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Thu, 3 Nov 2022 10:06:24 +1300
Subject: [PATCH] Support huge_pages and huge_page_size on FreeBSD.
FreeBSD often uses huge (super) pages due to automatic promotion, but it
may be useful to be able to request that explicitly, and to be able to
experiment with different page sizes by explicit request.
Discussion: https://postgr.es/m/3043b674-46d6-a8e9-3811-5a3007c8dceb%40ww-it.cn
---
doc/src/sgml/config.sgml | 6 ++--
src/backend/port/sysv_shmem.c | 58 ++++++++++++++++++++++++++++++-----
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 559eb898a9..cd65916bb5 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1678,9 +1678,9 @@ include_dir 'conf.d'
</para>
<para>
- At present, this setting is supported only on Linux and Windows. The
+ At present, this setting is supported only on Linux, FreeBSD and Windows. The
setting is ignored on other systems when set to
- <literal>try</literal>. On Linux, it is only supported when
+ <literal>try</literal>. On Linux and FreeBSD, it is only supported when
<varname>shared_memory_type</varname> is set to <literal>mmap</literal>
(the default).
</para>
@@ -1742,7 +1742,7 @@ include_dir 'conf.d'
about usage and support, see <xref linkend="linux-huge-pages"/>.
</para>
<para>
- Non-default settings are currently supported only on Linux.
+ Non-default settings are currently supported only on Linux and FreeBSD.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index 97ce7b7c49..0bb8c20c8d 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -576,8 +576,9 @@ GetHugePageSize(Size *hugepagesize, int *mmap_flags)
bool
check_huge_page_size(int *newval, void **extra, GucSource source)
{
-#if !(defined(MAP_HUGE_MASK) && defined(MAP_HUGE_SHIFT))
- /* Recent enough Linux only, for now. See GetHugePageSize(). */
+#if !(defined(MAP_HUGE_MASK) && defined(MAP_HUGE_SHIFT)) && \
+ !defined(SHM_LARGEPAGE_ALLOC_DEFAULT)
+ /* Recent enough Linux and FreeBSD only, for now. */
if (*newval != 0)
{
GUC_check_errdetail("huge_page_size must be 0 on this platform.");
@@ -601,12 +602,9 @@ CreateAnonymousSegment(Size *size)
void *ptr = MAP_FAILED;
int mmap_errno = 0;
-#ifndef MAP_HUGETLB
- /* PGSharedMemoryCreate should have dealt with this case */
- Assert(huge_pages != HUGE_PAGES_ON);
-#else
if (huge_pages == HUGE_PAGES_ON || huge_pages == HUGE_PAGES_TRY)
{
+#ifdef MAP_HUGETLB
/*
* Round up the request size to a suitable large value.
*/
@@ -624,8 +622,52 @@ CreateAnonymousSegment(Size *size)
if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED)
elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
allocsize);
- }
#endif
+#ifdef SHM_LARGEPAGE_ALLOC_DEFAULT
+ int nsizes;
+ size_t *page_sizes;
+ size_t page_size;
+ int page_size_index = -1;
+
+ /*
+ * Find the matching page size index, or if huge_page_size wasn't set,
+ * then skip the smallest size and take the next one after that.
+ */
+ nsizes = getpagesizes(NULL, 0);
+ page_sizes = palloc(nsizes * sizeof(*page_sizes));
+ getpagesizes(page_sizes, nsizes);
+ for (int i = 0; i < nsizes; ++i)
+ {
+ if (huge_page_size * 1024 == page_sizes[i] ||
+ (huge_page_size == 0 && i > 0))
+ {
+ page_size = page_sizes[i];
+ page_size_index = i;
+ if (allocsize % page_size != 0)
+ allocsize += page_size - (allocsize % page_size);
+ break;
+ }
+ }
+ pfree(page_sizes);
+ if (index >= 0)
+ {
+ int fd;
+
+ fd = shm_create_largepage(SHM_ANON, O_RDWR, page_size_index,
+ SHM_LARGEPAGE_ALLOC_DEFAULT, 0);
+ if (fd >= 0)
+ {
+ if (ftruncate(fd, allocsize) == 0)
+ {
+ ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
+ MAP_SHARED, fd, 0);
+ mmap_errno = errno;
+ }
+ close(fd);
+ }
+ }
+#endif
+ }
if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON)
{
@@ -709,7 +751,7 @@ PGSharedMemoryCreate(Size size,
DataDir)));
/* Complain if hugepages demanded but we can't possibly support them */
-#if !defined(MAP_HUGETLB)
+#if !defined(MAP_HUGETLB) && !defined(SHM_LARGEPAGE_ALLOC_DEFAULT)
if (huge_pages == HUGE_PAGES_ON)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
--
2.35.1
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Large Pages and Super Pages for PostgreSQL
2022-01-16 07:32 Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
2022-11-07 22:59 ` Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
@ 2022-11-30 07:29 ` ZHU XIAN WEN <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: ZHU XIAN WEN @ 2022-11-30 07:29 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; DEVOPS_WwIT <[email protected]>; +Cc: pgsql-hackers
Hi Thomas
Thank you very much for the work.
I just got latest FreeBSD 13.1 environment, and I'm going to test and
verify it.
so would you please rebase latest patch?
best wishes
Tony
On 2022/11/8 06:59, Thomas Munro wrote:
> On Sun, Jan 16, 2022 at 8:32 PM Thomas Munro <[email protected]> wrote:
>> On Sun, Jan 16, 2022 at 6:03 PM DEVOPS_WwIT <[email protected]> wrote:
>>> Solaris and FreeBSD supports large/super pages, and can be used
>>> automatically by applications.
>>>
>>> Seems Postgres can't use the large/super pages on Solaris and FreeBSD
>>> os(I think can't use the large/super page HPUX and AIX), is there anyone
>>> could take a look?
>> 3. FreeBSD: FreeBSD does transparently migrate PostgreSQL memory to
>> "super" pages quite well in my experience, but there is also a new
>> facility in FreeBSD 13 to ask for specific page sizes explicitly. I
>> wrote a quick and dirty patch to enable PostgreSQL's huge_pages and
>> huge_page_size settings to work with that interface, but I haven't yet
>> got as far as testing it very hard or proposing it... but here it is,
>> if you like experimental code[2].
> I was reminded to rebase that and tidy it up a bit, by recent
> discussion of page table magic in other threads. Documentation of
> these interfaces is sparse to put it mildly (I may try to improve that
> myself) but basically the terminology is "super" for pages subject to
> promotion/demotion, and "large" when explicitly managed. Not
> proposing for commit right now as I need to learn more about all this
> and there are some policy decisions lurking in here (eg synchronous
> defrag vs nowait depending on flags), but the patch may be useful for
> experimentation. For example, it allows huge_page_size=1GB if your
> system can handle that.
Attachments:
[application/pgp-keys] OpenPGP_0x6A40B0F18DBF5A23.asc (3.1K, ../../[email protected]/2-OpenPGP_0x6A40B0F18DBF5A23.asc)
download
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/3-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v2] Correctly update contfol file at the end of archive recovery
@ 2022-02-14 04:04 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-14 04:04 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Since all buffers have flushed out, we may safely regard that restart
point established regardless of the recovery state. Thus we may and
should update checkpoint LSNs of checkpoint file in that case. Still
we update minRecoveryPoint only during archive recovery but explicitly
clear if we have exited recovery.
Addition to that fix, this commit makes some cosmetic changes that
consist with the changes we are going to make on the master branch.
---
src/backend/access/transam/xlog.c | 81 ++++++++++++++++++++-----------
1 file changed, 53 insertions(+), 28 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6208e123e5..28c3c4b7cf 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9653,7 +9653,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9672,7 +9672,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9681,52 +9684,74 @@ CreateRestartPoint(int flags)
PriorRedoPtr = ControlFile->checkPointCopy.redo;
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Update pg_control, using current time if no later checkpoints have been
+ * performed.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (PriorRedoPtr < RedoRecPtr)
{
ControlFile->checkPoint = lastCheckPointRecPtr;
ControlFile->checkPointCopy = lastCheckPoint;
ControlFile->time = (pg_time_t) time(NULL);
/*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened
+ * already while writing out dirty buffers, but not necessarily -
+ * e.g. because no buffers were dirtied. We do this because a
+ * non-exclusive base backup uses minRecoveryPoint to determine which
+ * WAL files must be included in the backup, and the file (or files)
+ * containing the checkpoint record must be included, at a
+ * minimum. Note that for an ordinary restart of recovery there's no
+ * value in having the minimum recovery point any earlier than this
* anyway, because redo will begin just after the checkpoint record.
+ * This is a quick hack to make sure nothing really bad happens if
+ * somehow we get here after the end-of-recovery checkpoint.
*/
- if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
- ControlFile->minRecoveryPointTLI = lastCheckPoint.ThisTimeLineID;
+ if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
+ {
+ ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
+ ControlFile->minRecoveryPointTLI = lastCheckPoint.ThisTimeLineID;
- /* update local copy */
- minRecoveryPoint = ControlFile->minRecoveryPoint;
- minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+ /* update local copy */
+ minRecoveryPoint = ControlFile->minRecoveryPoint;
+ minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+ }
+ if (flags & CHECKPOINT_IS_SHUTDOWN)
+ ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
}
- if (flags & CHECKPOINT_IS_SHUTDOWN)
- ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
+ else
+ {
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+ }
+
UpdateControlFile();
}
LWLockRelease(ControlFileLock);
/*
* Update the average distance between checkpoints/restartpoints if the
- * prior checkpoint exists.
+ * prior checkpoint exists and we have advanced REDO LSN.
*/
- if (PriorRedoPtr != InvalidXLogRecPtr)
+ if (PriorRedoPtr != InvalidXLogRecPtr && RedoRecPtr > PriorRedoPtr)
UpdateCheckPointDistanceEstimate(RedoRecPtr - PriorRedoPtr);
+ /*
+ * Now we can safely regard this restart point as established.
+ *
+ * Since all buffers have been flushed so we regard this restart point as
+ * established. We could omit some of the following steps in the case
+ * where we have omitted control file updates, but we don't bother avoid
+ * them from performing since that case rarely happens and they don't harm
+ * even if they take place.
+ */
+
/*
* Delete old log files, those no longer needed for last restartpoint to
* prevent the disk holding the xlog from growing full.
@@ -9804,7 +9829,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Mon_Feb_14_14_40_22_2022_983)----
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v2] Correctly update contfol file at the end of archive recovery
@ 2022-02-14 04:04 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-14 04:04 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 ++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 26 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6208e123e5..ff4a90eacc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9587,6 +9587,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -9653,7 +9656,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9672,7 +9675,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9680,31 +9686,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9716,8 +9720,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9804,7 +9825,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Fri_Feb_25_15_31_12_2022_240)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0001-Correctly-update-contfol-file-at-the-end-of_v13.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v2] Correctly update contfol file at the end of archive recovery
@ 2022-02-14 04:04 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-14 04:04 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 ++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 26 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 6208e123e5..ff4a90eacc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9587,6 +9587,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -9653,7 +9656,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9672,7 +9675,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9680,31 +9686,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9716,8 +9720,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9804,7 +9825,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Wed_Mar_16_10_24_44_2022_775)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0001-Correctly-update-contfol-file-at-the-end-of_v13.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v2] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 05:46 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 05:46 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 ++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 26 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3d76fad128..3670ff81e7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9376,6 +9376,9 @@ CreateRestartPoint(int flags)
*/
LWLockAcquire(CheckpointLock, LW_EXCLUSIVE);
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -9445,7 +9448,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9461,7 +9464,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9469,31 +9475,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9505,8 +9509,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9590,7 +9611,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Fri_Feb_25_15_31_12_2022_240)----
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v2] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 05:46 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 05:46 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 ++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 26 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 3d76fad128..3670ff81e7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9376,6 +9376,9 @@ CreateRestartPoint(int flags)
*/
LWLockAcquire(CheckpointLock, LW_EXCLUSIVE);
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -9445,7 +9448,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9461,7 +9464,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9469,31 +9475,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9505,8 +9509,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9590,7 +9611,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Wed_Mar_16_10_24_44_2022_775)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0001-Correctly-update-contfol-file-at-the-end-of_v12-11.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v3] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 06:04 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 06:04 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 71 +++++++++++++++++++------------
1 file changed, 44 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 885558f291..2b2568c475 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9334,7 +9334,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9350,7 +9350,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9358,31 +9361,28 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9393,9 +9393,26 @@ CreateRestartPoint(int flags)
minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
- ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
+ ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9470,7 +9487,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Fri_Feb_25_16_47_01_2022_087)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v3-0001-Correctly-update-contfol-file-at-the-end-of_v10.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v3] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 06:04 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 06:04 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 71 +++++++++++++++++++------------
1 file changed, 44 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 885558f291..2b2568c475 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9334,7 +9334,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9350,7 +9350,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -9358,31 +9361,28 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9393,9 +9393,26 @@ CreateRestartPoint(int flags)
minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
- ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
+ ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9470,7 +9487,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Wed_Mar_16_10_24_44_2022_775)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0001-Correctly-update-contfol-file-at-the-end-of_v10.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v3] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 07:35 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 07:35 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 +++++++++++++++++++------------
1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c64febdb53..9fb66ad7d5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9434,7 +9434,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9450,7 +9450,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo pointer, used later to determine
@@ -9458,32 +9461,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->prevCheckPoint = ControlFile->checkPoint;
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still running. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->prevCheckPoint = ControlFile->checkPoint;
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9494,9 +9494,26 @@ CreateRestartPoint(int flags)
minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
- ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
+ ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9579,7 +9596,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("last completed transaction was at log time %s",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Wed_Mar_16_10_24_44_2022_775)----
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v3] Correctly update contfol file at the end of archive recovery
@ 2022-02-25 07:35 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-02-25 07:35 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 73 +++++++++++++++++++------------
1 file changed, 45 insertions(+), 28 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c64febdb53..9fb66ad7d5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9434,7 +9434,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -9450,7 +9450,10 @@ CreateRestartPoint(int flags)
if (log_checkpoints)
LogCheckpointStart(flags, true);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo pointer, used later to determine
@@ -9458,32 +9461,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->prevCheckPoint = ControlFile->checkPoint;
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still running. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->prevCheckPoint = ControlFile->checkPoint;
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
- ControlFile->time = (pg_time_t) time(NULL);
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -9494,9 +9494,26 @@ CreateRestartPoint(int flags)
minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
- ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
+ ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ minRecoveryPoint = InvalidXLogRecPtr;
+ minRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -9579,7 +9596,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- (uint32) (lastCheckPoint.redo >> 32), (uint32) lastCheckPoint.redo),
+ (uint32) (RedoRecPtr >> 32), (uint32) RedoRecPtr),
xtime ? errdetail("last completed transaction was at log time %s",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Fri_Feb_25_16_47_01_2022_087)----
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v12 1/5] Correctly update contfol file at the end of archive recovery
@ 2022-03-04 04:18 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-04 04:18 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 69 ++++++++++++++++++++-----------
1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0d2bd7a357..bd962763cc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6899,6 +6899,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -6964,7 +6967,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -6983,7 +6986,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -6991,30 +6997,26 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -7026,8 +7028,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ LocalMinRecoveryPoint = InvalidXLogRecPtr;
+ LocalMinRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -7104,7 +7123,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Tue_Mar_15_17_23_40_2022_661)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v12-0002-Add-checkpoint-and-redo-LSN-to-LogCheckpointEnd-.patch"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v11 1/5] Correctly update contfol file at the end of archive recovery
@ 2022-03-04 04:18 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-04 04:18 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 72 ++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 0d2bd7a357..3987aa81de 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6899,6 +6899,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -6964,7 +6967,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -6983,7 +6986,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -6991,30 +6997,29 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
+ /* Update control file using current time */
+ ControlFile->time = (pg_time_t) time(NULL);
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -7026,8 +7031,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ LocalMinRecoveryPoint = InvalidXLogRecPtr;
+ LocalMinRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -7104,7 +7126,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Fri_Mar__4_14_10_38_2022_481)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v11-0002-Add-checkpoint-and-redo-LSN-to-LogCheckpointEnd-.patch"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v4] Correctly update contfol file at the end of archive recovery
@ 2022-03-04 04:18 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-04 04:18 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 69 ++++++++++++++++++++-----------
1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ed16f279b1..dd9c564988 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6900,6 +6900,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -6965,7 +6968,7 @@ CreateRestartPoint(int flags)
/* Also update the info_lck-protected copy */
SpinLockAcquire(&XLogCtl->info_lck);
- XLogCtl->RedoRecPtr = lastCheckPoint.redo;
+ XLogCtl->RedoRecPtr = RedoRecPtr;
SpinLockRelease(&XLogCtl->info_lck);
/*
@@ -6984,7 +6987,10 @@ CreateRestartPoint(int flags)
/* Update the process title */
update_checkpoint_display(flags, true, false);
- CheckPointGuts(lastCheckPoint.redo, flags);
+ CheckPointGuts(RedoRecPtr, flags);
+
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
/*
* Remember the prior checkpoint's redo ptr for
@@ -6992,30 +6998,26 @@ CreateRestartPoint(int flags)
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record. This is a quick hack to make sure nothing really bad
+ * happens if somehow we get here after the end-of-recovery checkpoint.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a non-exclusive base backup uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -7027,8 +7029,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ LocalMinRecoveryPoint = InvalidXLogRecPtr;
+ LocalMinRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
@@ -7105,7 +7124,7 @@ CreateRestartPoint(int flags)
xtime = GetLatestXTime();
ereport((log_checkpoints ? LOG : DEBUG2),
(errmsg("recovery restart point at %X/%X",
- LSN_FORMAT_ARGS(lastCheckPoint.redo)),
+ LSN_FORMAT_ARGS(RedoRecPtr)),
xtime ? errdetail("Last completed transaction was at log time %s.",
timestamptz_to_str(xtime)) : 0));
--
2.27.0
----Next_Part(Wed_Mar_16_10_24_44_2022_775)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0001-Correctly-update-contfol-file-at-the-end-of_v14.txt"
^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v5] Correctly update contfol file at the end of archive recovery
@ 2022-04-27 01:41 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Kyotaro Horiguchi @ 2022-04-27 01:41 UTC (permalink / raw)
CreateRestartPoint runs WAL file cleanup basing on the checkpoint just
have finished in the function. If the database has exited
DB_IN_ARCHIVE_RECOVERY state when the function is going to update
control file, the function refrains from updating the file at all then
proceeds to WAL cleanup having the latest REDO LSN, which is now
inconsistent with the control file. As the result, the succeeding
cleanup procedure overly removes WAL files against the control file
and leaves unrecoverable database until the next checkpoint finishes.
Along with that fix, we remove a dead code path for the case some
other process ran a simultaneous checkpoint. It seems like just a
preventive measure but it's no longer useful because we are sure that
checkpoint is performed only by checkpointer except single process
mode.
---
src/backend/access/transam/xlog.c | 62 ++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 61cda56c6f..a03b8de593 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6921,6 +6921,9 @@ CreateRestartPoint(int flags)
XLogSegNo _logSegNo;
TimestampTz xtime;
+ /* we don't assume concurrent checkpoint/restartpoint to run */
+ Assert (!IsUnderPostmaster || MyBackendType == B_CHECKPOINTER);
+
/* Get a local copy of the last safe checkpoint record. */
SpinLockAcquire(&XLogCtl->info_lck);
lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
@@ -7007,36 +7010,34 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /* Update pg_control */
+ LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
*/
PriorRedoPtr = ControlFile->checkPointCopy.redo;
+ Assert (PriorRedoPtr < RedoRecPtr);
+
+ ControlFile->checkPoint = lastCheckPointRecPtr;
+ ControlFile->checkPointCopy = lastCheckPoint;
+
/*
- * Update pg_control, using current time. Check that it still shows
- * DB_IN_ARCHIVE_RECOVERY state and an older checkpoint, else do nothing;
- * this is a quick hack to make sure nothing really bad happens if somehow
- * we get here after the end-of-recovery checkpoint.
+ * Ensure minRecoveryPoint is past the checkpoint record while archive
+ * recovery is still ongoing. Normally, this will have happened already
+ * while writing out dirty buffers, but not necessarily - e.g. because no
+ * buffers were dirtied. We do this because a non-exclusive base backup
+ * uses minRecoveryPoint to determine which WAL files must be included in
+ * the backup, and the file (or files) containing the checkpoint record
+ * must be included, at a minimum. Note that for an ordinary restart of
+ * recovery there's no value in having the minimum recovery point any
+ * earlier than this anyway, because redo will begin just after the
+ * checkpoint record.
*/
- LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
- if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY &&
- ControlFile->checkPointCopy.redo < lastCheckPoint.redo)
+ if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
{
- ControlFile->checkPoint = lastCheckPointRecPtr;
- ControlFile->checkPointCopy = lastCheckPoint;
-
- /*
- * Ensure minRecoveryPoint is past the checkpoint record. Normally,
- * this will have happened already while writing out dirty buffers,
- * but not necessarily - e.g. because no buffers were dirtied. We do
- * this because a backup performed in recovery uses minRecoveryPoint to
- * determine which WAL files must be included in the backup, and the
- * file (or files) containing the checkpoint record must be included,
- * at a minimum. Note that for an ordinary restart of recovery there's
- * no value in having the minimum recovery point any earlier than this
- * anyway, because redo will begin just after the checkpoint record.
- */
if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
{
ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
@@ -7048,8 +7049,25 @@ CreateRestartPoint(int flags)
}
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
- UpdateControlFile();
}
+ else
+ {
+ /* recovery mode is not supposed to end during shutdown restartpoint */
+ Assert((flags & CHECKPOINT_IS_SHUTDOWN) == 0);
+
+ /*
+ * Aarchive recovery has ended. Crash recovery ever after should
+ * always recover to the end of WAL
+ */
+ ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
+ ControlFile->minRecoveryPointTLI = 0;
+
+ /* also update local copy */
+ LocalMinRecoveryPoint = InvalidXLogRecPtr;
+ LocalMinRecoveryPointTLI = 0;
+ }
+
+ UpdateControlFile();
LWLockRelease(ControlFileLock);
/*
--
2.27.0
----Next_Part(Wed_Apr_27_10_43_53_2022_048)----
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2022-11-30 07:29 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-11-02 07:30 Re: Small run-time pruning doc fix Peter Eisentraut <[email protected]>
2018-11-04 05:23 ` David Rowley <[email protected]>
2018-11-12 13:46 ` Peter Eisentraut <[email protected]>
2018-11-12 21:20 ` David Rowley <[email protected]>
2022-01-16 07:32 Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
2022-11-07 22:59 ` Re: Large Pages and Super Pages for PostgreSQL Thomas Munro <[email protected]>
2022-11-30 07:29 ` Re: Large Pages and Super Pages for PostgreSQL ZHU XIAN WEN <[email protected]>
2022-02-14 04:04 [PATCH v2] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-14 04:04 [PATCH v2] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-14 04:04 [PATCH v2] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 05:46 [PATCH v2] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 05:46 [PATCH v2] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 06:04 [PATCH v3] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 06:04 [PATCH v3] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 07:35 [PATCH v3] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-02-25 07:35 [PATCH v3] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-03-04 04:18 [PATCH v12 1/5] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-03-04 04:18 [PATCH v11 1/5] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-03-04 04:18 [PATCH v4] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
2022-04-27 01:41 [PATCH v5] Correctly update contfol file at the end of archive recovery Kyotaro Horiguchi <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox