pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feeduse CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
21+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2024-07-08 64f34eb2e2 Use CREATE DATABASE ... STRATEGY = FILE_COPY in pg_upgrade.
* use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-04 19:39 Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Nathan Bossart @ 2024-06-04 19:39 UTC (permalink / raw)
To: pgsql-hackers
I noticed that the "Restoring database schemas in the new cluster" part of
pg_upgrade can take a while if you have many databases, so I experimented
with a couple different settings to see if there are any easy ways to speed
it up. The FILE_COPY strategy for CREATE DATABASE helped quite
significantly on my laptop. For ~3k empty databases, this step went from
~100 seconds to ~30 seconds with the attached patch. I see commit ad43a41
made a similar change for initdb, so there might even be an argument for
back-patching this to v15 (where STRATEGY was introduced). One thing I
still need to verify is that this doesn't harm anything when there are lots
of objects in the databases, i.e., more WAL generated during many
concurrent CREATE-DATABASE-induced checkpoints.
Thoughts?
--
nathan
Attachments:
use_file_copy_for_pg_upgrade.patch (text/plain, 514B) download
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e324070828..bc9fef4198 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3126,7 +3126,9 @@ dumpDatabase(Archive *fout)
*/
if (dopt->binary_upgrade)
{
- appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0 OID = %u",
+ appendPQExpBuffer(creaQry,
+ "CREATE DATABASE %s WITH TEMPLATE = template0 "
+ "OID = %u STRATEGY = FILE_COPY",
qdatname, dbCatId.oid);
}
else
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-05 16:47 Ranier Vilela <ranier.vf@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Ranier Vilela @ 2024-06-05 16:47 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: pgsql-hackers
Em ter., 4 de jun. de 2024 às 16:39, Nathan Bossart <
nathandbossart@gmail.com> escreveu:
> I noticed that the "Restoring database schemas in the new cluster" part of
> pg_upgrade can take a while if you have many databases, so I experimented
> with a couple different settings to see if there are any easy ways to speed
> it up. The FILE_COPY strategy for CREATE DATABASE helped quite
> significantly on my laptop. For ~3k empty databases, this step went from
> ~100 seconds to ~30 seconds with the attached patch. I see commit ad43a41
> made a similar change for initdb, so there might even be an argument for
> back-patching this to v15 (where STRATEGY was introduced). One thing I
> still need to verify is that this doesn't harm anything when there are lots
> of objects in the databases, i.e., more WAL generated during many
> concurrent CREATE-DATABASE-induced checkpoints.
>
> Thoughts?
>
Why not use it too, if not binary_upgrade?
else
{
appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0
STRATEGY = FILE_COPY",
qdatname);
}
It seems to me that it also improves in any use.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-05 17:07 Nathan Bossart <nathandbossart@gmail.com>
parent: Ranier Vilela <ranier.vf@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Nathan Bossart @ 2024-06-05 17:07 UTC (permalink / raw)
To: Ranier Vilela <ranier.vf@gmail.com>; +Cc: pgsql-hackers
On Wed, Jun 05, 2024 at 01:47:09PM -0300, Ranier Vilela wrote:
> Why not use it too, if not binary_upgrade?
>
> else
> {
> appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0
> STRATEGY = FILE_COPY",
> qdatname);
> }
>
> It seems to me that it also improves in any use.
Well, if that is true, and I'm not sure it is, then we should probably
consider changing the default STRATEGY in the server instead. I haven't
looked too deeply, but my assumption is that when fsync is disabled (as it
is when restoring schemas during pg_upgrade), both checkpointing and
copying the template database are sufficiently fast enough to beat writing
out all the data to WAL. Furthermore, in my test, all the databases were
basically empty, so I suspect that some CREATE DATABASE commands could
piggy-back on checkpoints initiated by others. This might not be the case
when there are many objects in each database, and that is a scenario I have
yet to test.
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-05 17:28 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Ranier Vilela <ranier.vf@gmail.com>
1 sibling, 2 replies; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-05 17:28 UTC (permalink / raw)
To: Ranier Vilela <ranier.vf@gmail.com>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Wed, 5 Jun 2024 at 18:47, Ranier Vilela <ranier.vf@gmail.com> wrote:
>
> Em ter., 4 de jun. de 2024 às 16:39, Nathan Bossart <nathandbossart@gmail.com> escreveu:
>>
>> I noticed that the "Restoring database schemas in the new cluster" part of
>> pg_upgrade can take a while if you have many databases, so I experimented
>> with a couple different settings to see if there are any easy ways to speed
>> it up. The FILE_COPY strategy for CREATE DATABASE helped quite
>> significantly on my laptop. For ~3k empty databases, this step went from
>> ~100 seconds to ~30 seconds with the attached patch. I see commit ad43a41
>> made a similar change for initdb, so there might even be an argument for
>> back-patching this to v15 (where STRATEGY was introduced). One thing I
>> still need to verify is that this doesn't harm anything when there are lots
>> of objects in the databases, i.e., more WAL generated during many
>> concurrent CREATE-DATABASE-induced checkpoints.
>>
>> Thoughts?
>
> Why not use it too, if not binary_upgrade?
Because in the normal case (not during binary_upgrade) you don't want
to have to generate 2 checkpoints for every created database,
especially not when your shared buffers are large. Checkpoints' costs
scale approximately linearly with the size of shared buffers, so being
able to skip those checkpoints (with strategy=WAL_LOG) will save a lot
of performance in the systems where this performance impact matters
most.
>> I noticed that the "Restoring database schemas in the new cluster" part of
>> pg_upgrade can take a while if you have many databases, so I experimented
>> with a couple different settings to see if there are any easy ways to speed
>> it up. The FILE_COPY strategy for CREATE DATABASE helped quite
>> significantly on my laptop. For ~3k empty databases, this step went from
>> ~100 seconds to ~30 seconds with the attached patch.
As for "on my laptop", that sounds very reasonable, but could you
check the performance on systems with larger shared buffer
configurations? I'd imagine (but haven't checked) that binary upgrade
target systems may already be using the shared_buffers from their
source system, which would cause a severe regression when the
to-be-upgraded system has large shared buffers. For initdb the
database size is known in advance and shared_buffers is approximately
empty, but the same is not (always) true when we're doing binary
upgrades.
Kind regards,
Matthias van de Meent
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-05 17:45 Nathan Bossart <nathandbossart@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Nathan Bossart @ 2024-06-05 17:45 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Wed, Jun 05, 2024 at 07:28:42PM +0200, Matthias van de Meent wrote:
> As for "on my laptop", that sounds very reasonable, but could you
> check the performance on systems with larger shared buffer
> configurations? I'd imagine (but haven't checked) that binary upgrade
> target systems may already be using the shared_buffers from their
> source system, which would cause a severe regression when the
> to-be-upgraded system has large shared buffers. For initdb the
> database size is known in advance and shared_buffers is approximately
> empty, but the same is not (always) true when we're doing binary
> upgrades.
Will do. FWIW I haven't had much luck improving pg_upgrade times by
adjusting server settings, but I also haven't explored it all that much.
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-07 05:18 Dilip Kumar <dilipbalaut@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Dilip Kumar @ 2024-06-07 05:18 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Wed, Jun 5, 2024 at 10:59 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Wed, 5 Jun 2024 at 18:47, Ranier Vilela <ranier.vf@gmail.com> wrote:
> >
> > Em ter., 4 de jun. de 2024 às 16:39, Nathan Bossart <nathandbossart@gmail.com> escreveu:
> >>
> >> I noticed that the "Restoring database schemas in the new cluster" part of
> >> pg_upgrade can take a while if you have many databases, so I experimented
> >> with a couple different settings to see if there are any easy ways to speed
> >> it up. The FILE_COPY strategy for CREATE DATABASE helped quite
> >> significantly on my laptop. For ~3k empty databases, this step went from
> >> ~100 seconds to ~30 seconds with the attached patch. I see commit ad43a41
> >> made a similar change for initdb, so there might even be an argument for
> >> back-patching this to v15 (where STRATEGY was introduced). One thing I
> >> still need to verify is that this doesn't harm anything when there are lots
> >> of objects in the databases, i.e., more WAL generated during many
> >> concurrent CREATE-DATABASE-induced checkpoints.
> >>
> >> Thoughts?
> >
> > Why not use it too, if not binary_upgrade?
>
> Because in the normal case (not during binary_upgrade) you don't want
> to have to generate 2 checkpoints for every created database,
> especially not when your shared buffers are large. Checkpoints' costs
> scale approximately linearly with the size of shared buffers, so being
> able to skip those checkpoints (with strategy=WAL_LOG) will save a lot
> of performance in the systems where this performance impact matters
> most.
I agree with you that we introduced the WAL_LOG strategy to avoid
these force checkpoints. However, in binary upgrade cases where no
operations are happening in the system, the FILE_COPY strategy should
be faster.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-07 06:27 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-07 06:27 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Fri, 7 Jun 2024 at 07:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, Jun 5, 2024 at 10:59 PM Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
>>
>> On Wed, 5 Jun 2024 at 18:47, Ranier Vilela <ranier.vf@gmail.com> wrote:
>>>
>>> Why not use it too, if not binary_upgrade?
>>
>> Because in the normal case (not during binary_upgrade) you don't want
>> to have to generate 2 checkpoints for every created database,
>> especially not when your shared buffers are large. Checkpoints' costs
>> scale approximately linearly with the size of shared buffers, so being
>> able to skip those checkpoints (with strategy=WAL_LOG) will save a lot
>> of performance in the systems where this performance impact matters
>> most.
>
> I agree with you that we introduced the WAL_LOG strategy to avoid
> these force checkpoints. However, in binary upgrade cases where no
> operations are happening in the system, the FILE_COPY strategy should
> be faster.
While you would be correct if there were no operations happening in
the system, during binary upgrade we're still actively modifying
catalogs; and this is done with potentially many concurrent jobs. I
think it's not unlikely that this would impact performance.
Now that I think about it, arguably, we shouldn't need to run
checkpoints during binary upgrade for the FILE_COPY strategy after
we've restored the template1 database and created a checkpoint after
that: All other databases use template1 as their template database,
and the checkpoint is there mostly to guarantee the FS knows about all
changes in the template database before we task it with copying the
template database over to our new database, so the protections we get
from more checkpoints are practically useless.
If such a change were implemented (i.e. no checkpoints for FILE_COPY
in binary upgrade, with a single manual checkpoint after restoring
template1 in create_new_objects) I think most of my concerns with this
patch would be alleviated.
Kind regards,
Matthias van de Meent
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-07 08:27 Dilip Kumar <dilipbalaut@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Dilip Kumar @ 2024-06-07 08:27 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Fri, Jun 7, 2024 at 11:57 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Fri, 7 Jun 2024 at 07:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, Jun 5, 2024 at 10:59 PM Matthias van de Meent
> > <boekewurm+postgres@gmail.com> wrote:
> >>
> >> On Wed, 5 Jun 2024 at 18:47, Ranier Vilela <ranier.vf@gmail.com> wrote:
> >>>
> >>> Why not use it too, if not binary_upgrade?
> >>
> >> Because in the normal case (not during binary_upgrade) you don't want
> >> to have to generate 2 checkpoints for every created database,
> >> especially not when your shared buffers are large. Checkpoints' costs
> >> scale approximately linearly with the size of shared buffers, so being
> >> able to skip those checkpoints (with strategy=WAL_LOG) will save a lot
> >> of performance in the systems where this performance impact matters
> >> most.
> >
> > I agree with you that we introduced the WAL_LOG strategy to avoid
> > these force checkpoints. However, in binary upgrade cases where no
> > operations are happening in the system, the FILE_COPY strategy should
> > be faster.
>
> While you would be correct if there were no operations happening in
> the system, during binary upgrade we're still actively modifying
> catalogs; and this is done with potentially many concurrent jobs. I
> think it's not unlikely that this would impact performance.
Maybe, but generally, long checkpoints are problematic because they
involve a lot of I/O, which hampers overall system performance.
However, in the case of a binary upgrade, the concurrent operations
are only performing a schema restore, not a real data restore.
Therefore, it shouldn't have a significant impact, and the checkpoints
should also not do a lot of I/O during binary upgrade, right?
> Now that I think about it, arguably, we shouldn't need to run
> checkpoints during binary upgrade for the FILE_COPY strategy after
> we've restored the template1 database and created a checkpoint after
> that: All other databases use template1 as their template database,
> and the checkpoint is there mostly to guarantee the FS knows about all
> changes in the template database before we task it with copying the
> template database over to our new database, so the protections we get
> from more checkpoints are practically useless.
> If such a change were implemented (i.e. no checkpoints for FILE_COPY
> in binary upgrade, with a single manual checkpoint after restoring
> template1 in create_new_objects) I think most of my concerns with this
> patch would be alleviated.
Yeah, I think that's a valid point. The second checkpoint is to ensure
that the XLOG_DBASE_CREATE_FILE_COPY never gets replayed. However, for
binary upgrades, we don't need that guarantee because a checkpoint
will be performed during shutdown at the end of the upgrade anyway.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-07 09:10 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-07 09:10 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Fri, 7 Jun 2024 at 10:28, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Jun 7, 2024 at 11:57 AM Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
>>
>> On Fri, 7 Jun 2024 at 07:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>>
>>> On Wed, Jun 5, 2024 at 10:59 PM Matthias van de Meent
>>> <boekewurm+postgres@gmail.com> wrote:
>>>
>>> I agree with you that we introduced the WAL_LOG strategy to avoid
>>> these force checkpoints. However, in binary upgrade cases where no
>>> operations are happening in the system, the FILE_COPY strategy should
>>> be faster.
>>
>> While you would be correct if there were no operations happening in
>> the system, during binary upgrade we're still actively modifying
>> catalogs; and this is done with potentially many concurrent jobs. I
>> think it's not unlikely that this would impact performance.
>
> Maybe, but generally, long checkpoints are problematic because they
> involve a lot of I/O, which hampers overall system performance.
> However, in the case of a binary upgrade, the concurrent operations
> are only performing a schema restore, not a real data restore.
> Therefore, it shouldn't have a significant impact, and the checkpoints
> should also not do a lot of I/O during binary upgrade, right?
My primary concern isn't the IO, but the O(shared_buffers) that we
have to go through during a checkpoint. As I mentioned upthread, it is
reasonably possible the new cluster is already setup with a good
fraction of the old system's shared_buffers configured. Every
checkpoint has to scan all those buffers, which IMV can get (much)
more expensive than the IO overhead caused by the WAL_LOG strategy. It
may be a baseless fear as I haven't done the performance benchmarks
for this, but I wouldn't be surprised if shared_buffers=8GB would
measurably impact the upgrade performance in the current patch (vs the
default 128MB).
I'll note that the documentation for upgrading with pg_upgrade has the
step for updating postgresql.conf / postgresql.auto.conf only after
pg_upgrade has run already, but that may not be how it's actually
used: after all, we don't have full control in this process, the user
is the one who provides the new cluster with initdb.
>> If such a change were implemented (i.e. no checkpoints for FILE_COPY
>> in binary upgrade, with a single manual checkpoint after restoring
>> template1 in create_new_objects) I think most of my concerns with this
>> patch would be alleviated.
>
> Yeah, I think that's a valid point. The second checkpoint is to ensure
> that the XLOG_DBASE_CREATE_FILE_COPY never gets replayed. However, for
> binary upgrades, we don't need that guarantee because a checkpoint
> will be performed during shutdown at the end of the upgrade anyway.
Indeed.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-07 09:17 Dilip Kumar <dilipbalaut@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Dilip Kumar @ 2024-06-07 09:17 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Ranier Vilela <ranier.vf@gmail.com>; Nathan Bossart <nathandbossart@gmail.com>; pgsql-hackers
On Fri, Jun 7, 2024 at 2:40 PM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Fri, 7 Jun 2024 at 10:28, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Fri, Jun 7, 2024 at 11:57 AM Matthias van de Meent
> > <boekewurm+postgres@gmail.com> wrote:
> >>
> >> On Fri, 7 Jun 2024 at 07:18, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >>>
> >>> On Wed, Jun 5, 2024 at 10:59 PM Matthias van de Meent
> >>> <boekewurm+postgres@gmail.com> wrote:
> >>>
> >>> I agree with you that we introduced the WAL_LOG strategy to avoid
> >>> these force checkpoints. However, in binary upgrade cases where no
> >>> operations are happening in the system, the FILE_COPY strategy should
> >>> be faster.
> >>
> >> While you would be correct if there were no operations happening in
> >> the system, during binary upgrade we're still actively modifying
> >> catalogs; and this is done with potentially many concurrent jobs. I
> >> think it's not unlikely that this would impact performance.
> >
> > Maybe, but generally, long checkpoints are problematic because they
> > involve a lot of I/O, which hampers overall system performance.
> > However, in the case of a binary upgrade, the concurrent operations
> > are only performing a schema restore, not a real data restore.
> > Therefore, it shouldn't have a significant impact, and the checkpoints
> > should also not do a lot of I/O during binary upgrade, right?
>
> My primary concern isn't the IO, but the O(shared_buffers) that we
> have to go through during a checkpoint. As I mentioned upthread, it is
> reasonably possible the new cluster is already setup with a good
> fraction of the old system's shared_buffers configured. Every
> checkpoint has to scan all those buffers, which IMV can get (much)
> more expensive than the IO overhead caused by the WAL_LOG strategy. It
> may be a baseless fear as I haven't done the performance benchmarks
> for this, but I wouldn't be surprised if shared_buffers=8GB would
> measurably impact the upgrade performance in the current patch (vs the
> default 128MB).
Okay, that's a valid point.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-11 02:01 Nathan Bossart <nathandbossart@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Nathan Bossart @ 2024-06-11 02:01 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Fri, Jun 07, 2024 at 11:10:25AM +0200, Matthias van de Meent wrote:
> My primary concern isn't the IO, but the O(shared_buffers) that we
> have to go through during a checkpoint. As I mentioned upthread, it is
> reasonably possible the new cluster is already setup with a good
> fraction of the old system's shared_buffers configured. Every
> checkpoint has to scan all those buffers, which IMV can get (much)
> more expensive than the IO overhead caused by the WAL_LOG strategy. It
> may be a baseless fear as I haven't done the performance benchmarks
> for this, but I wouldn't be surprised if shared_buffers=8GB would
> measurably impact the upgrade performance in the current patch (vs the
> default 128MB).
I did a handful of benchmarks on an r5.24xlarge that seem to prove your
point. The following are the durations of the pg_restore step of
pg_upgrade:
* 10k empty databases, 128MB shared_buffers
WAL_LOG: 1m 01s
FILE_COPY: 0m 22s
* 10k empty databases, 100GB shared_buffers
WAL_LOG: 2m 03s
FILE_COPY: 5m 08s
* 2.5k databases with 10k tables each, 128MB shared_buffers
WAL_LOG: 17m 20s
FILE_COPY: 16m 44s
* 2.5k databases with 10k tables each, 100GB shared_buffers
WAL_LOG: 16m 39s
FILE_COPY: 15m 21s
I was surprised with the last result, but there's enough other stuff
happening during such a test that I hesitate to conclude much.
> I'll note that the documentation for upgrading with pg_upgrade has the
> step for updating postgresql.conf / postgresql.auto.conf only after
> pg_upgrade has run already, but that may not be how it's actually
> used: after all, we don't have full control in this process, the user
> is the one who provides the new cluster with initdb.
Good point. I think it's clear that FILE_COPY is not necessarily a win in
all cases for pg_upgrade.
>>> If such a change were implemented (i.e. no checkpoints for FILE_COPY
>>> in binary upgrade, with a single manual checkpoint after restoring
>>> template1 in create_new_objects) I think most of my concerns with this
>>> patch would be alleviated.
>>
>> Yeah, I think that's a valid point. The second checkpoint is to ensure
>> that the XLOG_DBASE_CREATE_FILE_COPY never gets replayed. However, for
>> binary upgrades, we don't need that guarantee because a checkpoint
>> will be performed during shutdown at the end of the upgrade anyway.
>
> Indeed.
It looks like pg_dump always uses template0, so AFAICT we don't even need
the suggested manual checkpoint after restoring template1.
I do like the idea of skipping a bunch of unnecessary operations in binary
upgrade mode, since it'll help me in my goal of speeding up pg_upgrade.
But I'm a bit hesitant to get too fancy here and to introduce a bunch of
new "if (IsBinaryUpgrade)" checks if the gains in the field won't be all
that exciting. However, we've already sprinkled such checks quite
liberally, so maybe I'm being too cautious...
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-11 08:39 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-11 08:39 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Tue, 11 Jun 2024 at 04:01, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Fri, Jun 07, 2024 at 11:10:25AM +0200, Matthias van de Meent wrote:
> > My primary concern isn't the IO, but the O(shared_buffers) that we
> > have to go through during a checkpoint. As I mentioned upthread, it is
> > reasonably possible the new cluster is already setup with a good
> > fraction of the old system's shared_buffers configured. Every
> > checkpoint has to scan all those buffers, which IMV can get (much)
> > more expensive than the IO overhead caused by the WAL_LOG strategy. It
> > may be a baseless fear as I haven't done the performance benchmarks
> > for this, but I wouldn't be surprised if shared_buffers=8GB would
> > measurably impact the upgrade performance in the current patch (vs the
> > default 128MB).
>
> I did a handful of benchmarks on an r5.24xlarge that seem to prove your
> point. The following are the durations of the pg_restore step of
> pg_upgrade:
>
> * 10k empty databases, 128MB shared_buffers
> WAL_LOG: 1m 01s
> FILE_COPY: 0m 22s
>
> * 10k empty databases, 100GB shared_buffers
> WAL_LOG: 2m 03s
> FILE_COPY: 5m 08s
>
> * 2.5k databases with 10k tables each, 128MB shared_buffers
> WAL_LOG: 17m 20s
> FILE_COPY: 16m 44s
>
> * 2.5k databases with 10k tables each, 100GB shared_buffers
> WAL_LOG: 16m 39s
> FILE_COPY: 15m 21s
>
> I was surprised with the last result, but there's enough other stuff
> happening during such a test that I hesitate to conclude much.
If you still have the test data set up, could you test the attached
patch (which does skip the checkpoints in FILE_COPY mode during binary
upgrades)?
>>>> If such a change were implemented (i.e. no checkpoints for FILE_COPY
>>>> in binary upgrade, with a single manual checkpoint after restoring
>>>> template1 in create_new_objects) I think most of my concerns with this
>>>> patch would be alleviated.
>>>
>>> Yeah, I think that's a valid point. The second checkpoint is to ensure
>>> that the XLOG_DBASE_CREATE_FILE_COPY never gets replayed. However, for
>>> binary upgrades, we don't need that guarantee because a checkpoint
>>> will be performed during shutdown at the end of the upgrade anyway.
>>
>> Indeed.
>
> It looks like pg_dump always uses template0, so AFAICT we don't even need
> the suggested manual checkpoint after restoring template1.
Thanks for reminding me. It seems I misunderstood the reason why we
first process template1 in create_new_objects, as I didn't read the
comments thoroughly enough.
> I do like the idea of skipping a bunch of unnecessary operations in binary
> upgrade mode, since it'll help me in my goal of speeding up pg_upgrade.
> But I'm a bit hesitant to get too fancy here and to introduce a bunch of
> new "if (IsBinaryUpgrade)" checks if the gains in the field won't be all
> that exciting. However, we've already sprinkled such checks quite
> liberally, so maybe I'm being too cautious...
Hmm, yes. From an IO perspective I think this could be an improvement,
but let's check the numbers first.
Kind regards,
Matthias van de Meent
Attachments:
0001-pg_upgrade-use-CREATE-DATABASE-.-STRATEGY-FILE_COPY.patch (application/octet-stream, 3.4K) download
From 3cce74e0a8124e2c7c3745e961b99e4700677692 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Tue, 11 Jun 2024 10:24:00 +0200
Subject: [PATCH] pg_upgrade: use CREATE DATABASE ... STRATEGY=FILE_COPY
While the strategy is considered very costly due to expensive checkpoints
when the target system has configured large shared_buffers, the issues that
those checkpoints protect against don't apply in a pg_upgrade system.
By skipping the checkpoints during CREATE DATABASE ... STRATEGY=FILE_COPY
and using that strategy for pg_upgrade, we can save a lot of time and IO
that would otherwise be spent on reading pages from disk, generating WAL
records, writing those WAL records to disk, and then writing the pages to
disk - which has the potential of at least double the IO of FILE_COPY.
Author: Nathan Bossart
Author: Matthias van de Meent
---
src/backend/commands/dbcommands.c | 20 +++++++++++++++++---
src/bin/pg_dump/pg_dump.c | 4 +++-
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index be629ea92c..04ad6f0b34 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -563,9 +563,16 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,
* happened while we're copying files, a file might be deleted just when
* we're about to copy it, causing the lstat() call in copydir() to fail
* with ENOENT.
+ *
+ * While we're in binary upgrade mode we have full control over which
+ * databases we access. As the template database is always going to be
+ * template0, which doesn't allow connections, we can be sure that that
+ * database won't have any modified pages in shared memory, thus we can
+ * omit this checkpoint.
*/
- RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
- CHECKPOINT_WAIT | CHECKPOINT_FLUSH_ALL);
+ if (!IsBinaryUpgrade)
+ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
+ CHECKPOINT_WAIT | CHECKPOINT_FLUSH_ALL);
/*
* Iterate through all tablespaces of the template database, and copy each
@@ -657,10 +664,17 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,
* seem to be much we can do about that except document it as a
* limitation.
*
+ * In binary upgrade mode, neither of these are a concern: generated WAL
+ * is dropped, and the template0 database (which is the only source
+ * database used in binary upgrade) can't have concurrent modifications,
+ * so we skip this checkpoint.
+ *
* See CreateDatabaseUsingWalLog() for a less cheesy CREATE DATABASE
* strategy that avoids these problems.
*/
- RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
+ if (!IsBinaryUpgrade)
+ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
+ CHECKPOINT_WAIT);
}
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index cb14fcafea..aec702675c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3126,7 +3126,9 @@ dumpDatabase(Archive *fout)
*/
if (dopt->binary_upgrade)
{
- appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0 OID = %u",
+ appendPQExpBuffer(creaQry,
+ "CREATE DATABASE %s WITH TEMPLATE = template0 OID = %u "
+ "STRATEGY = FILE_COPY",
qdatname, dbCatId.oid);
}
else
--
2.40.1
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-11 16:03 Nathan Bossart <nathandbossart@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Nathan Bossart @ 2024-06-11 16:03 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Tue, Jun 11, 2024 at 10:39:51AM +0200, Matthias van de Meent wrote:
> On Tue, 11 Jun 2024 at 04:01, Nathan Bossart <nathandbossart@gmail.com> wrote:
>> I did a handful of benchmarks on an r5.24xlarge that seem to prove your
>> point. The following are the durations of the pg_restore step of
>> pg_upgrade:
>>
>> * 10k empty databases, 128MB shared_buffers
>> WAL_LOG: 1m 01s
>> FILE_COPY: 0m 22s
>>
>> * 10k empty databases, 100GB shared_buffers
>> WAL_LOG: 2m 03s
>> FILE_COPY: 5m 08s
>>
>> * 2.5k databases with 10k tables each, 128MB shared_buffers
>> WAL_LOG: 17m 20s
>> FILE_COPY: 16m 44s
>>
>> * 2.5k databases with 10k tables each, 100GB shared_buffers
>> WAL_LOG: 16m 39s
>> FILE_COPY: 15m 21s
>>
>> I was surprised with the last result, but there's enough other stuff
>> happening during such a test that I hesitate to conclude much.
>
> If you still have the test data set up, could you test the attached
> patch (which does skip the checkpoints in FILE_COPY mode during binary
> upgrades)?
With your patch, I see the following:
* 10k empty databases, 128MB shared_buffers: 0m 27s
* 10k empty databases, 100GB shared_buffers: 1m 44s
I believe the reason the large buffer cache test is still quite a bit
slower is due to the truncation of pg_largeobject (specifically its call to
DropRelationsAllBuffers()). This TRUNCATE command was added in commit
bbe08b8.
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-12 14:41 Nathan Bossart <nathandbossart@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Nathan Bossart @ 2024-06-12 14:41 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Tue, Jun 11, 2024 at 10:39:51AM +0200, Matthias van de Meent wrote:
> On Tue, 11 Jun 2024 at 04:01, Nathan Bossart <nathandbossart@gmail.com> wrote:
>> It looks like pg_dump always uses template0, so AFAICT we don't even need
>> the suggested manual checkpoint after restoring template1.
>
> Thanks for reminding me. It seems I misunderstood the reason why we
> first process template1 in create_new_objects, as I didn't read the
> comments thoroughly enough.
Actually, I think you are right that we need a manual checkpoint, except I
think we need it to be after prepare_new_globals(). set_frozenxids()
connects to each database (including template0) and updates a bunch of
pg_class rows, and we probably want those on disk before we start copying
the files to create all the user's databases.
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-14 21:29 Nathan Bossart <nathandbossart@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 2 replies; 21+ messages in thread
From: Nathan Bossart @ 2024-06-14 21:29 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Wed, Jun 12, 2024 at 09:41:01AM -0500, Nathan Bossart wrote:
> Actually, I think you are right that we need a manual checkpoint, except I
> think we need it to be after prepare_new_globals(). set_frozenxids()
> connects to each database (including template0) and updates a bunch of
> pg_class rows, and we probably want those on disk before we start copying
> the files to create all the user's databases.
Here is an updated patch.
--
nathan
Attachments:
v3-0001-Use-CREATE-DATABASE-.-STRATEGY-FILE_COPY-in-pg_up.patch (text/plain, 4.8K) download
From c3aeed33b351475896c2b4002cff125f1edeeb42 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 14 Jun 2024 16:25:04 -0500
Subject: [PATCH v3 1/1] Use CREATE DATABASE ... STRATEGY = FILE_COPY in
pg_upgrade.
While the strategy is considered very costly due to expensive checkpoints
when the target system has configured large shared_buffers, the issues that
those checkpoints protect against don't apply in a pg_upgrade system.
By skipping the checkpoints during CREATE DATABASE ... STRATEGY=FILE_COPY
and using that strategy for pg_upgrade, we can save a lot of time and IO
that would otherwise be spent on reading pages from disk, generating WAL
records, writing those WAL records to disk, and then writing the pages to
disk - which has the potential of at least double the IO of FILE_COPY.
Co-authored-by: Matthias van de Meent
Reviewed-by: Ranier Vilela, Dilip Kumar
Discussion: https://postgr.es/m/Zl9ta3FtgdjizkJ5%40nathan
---
src/backend/commands/dbcommands.c | 18 +++++++++++++++---
src/bin/pg_dump/pg_dump.c | 9 ++++++++-
src/bin/pg_upgrade/pg_upgrade.c | 12 ++++++++++++
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index be629ea92c..99d7a9ba3c 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -563,9 +563,14 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,
* happened while we're copying files, a file might be deleted just when
* we're about to copy it, causing the lstat() call in copydir() to fail
* with ENOENT.
+ *
+ * In binary upgrade mode, we can skip this checkpoint because we are
+ * careful to ensure that template0 is fully written to disk prior to any
+ * CREATE DATABASE commands.
*/
- RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
- CHECKPOINT_WAIT | CHECKPOINT_FLUSH_ALL);
+ if (!IsBinaryUpgrade)
+ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
+ CHECKPOINT_WAIT | CHECKPOINT_FLUSH_ALL);
/*
* Iterate through all tablespaces of the template database, and copy each
@@ -657,10 +662,17 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,
* seem to be much we can do about that except document it as a
* limitation.
*
+ * In binary upgrade mode, we can skip this checkpoint because neither of
+ * these problems applies: we don't ever replay the WAL generated during
+ * pg_upgrade, and we don't concurrently modify template0 (not to mention
+ * that trying to take a backup during pg_upgrade is pointless).
+ *
* See CreateDatabaseUsingWalLog() for a less cheesy CREATE DATABASE
* strategy that avoids these problems.
*/
- RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT);
+ if (!IsBinaryUpgrade)
+ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE |
+ CHECKPOINT_WAIT);
}
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e324070828..1e51a63442 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3123,10 +3123,17 @@ dumpDatabase(Archive *fout)
* since those can't be altered later. Other DB properties are left to
* the DATABASE PROPERTIES entry, so that they can be applied after
* reconnecting to the target DB.
+ *
+ * For binary upgrade, we use the FILE_COPY strategy because testing has
+ * shown it to be faster. When the server is in binary upgrade mode, it
+ * will also skip the checkpoints normally triggered by CREATE DATABASE
+ * STRATEGY = FILE_COPY.
*/
if (dopt->binary_upgrade)
{
- appendPQExpBuffer(creaQry, "CREATE DATABASE %s WITH TEMPLATE = template0 OID = %u",
+ appendPQExpBuffer(creaQry,
+ "CREATE DATABASE %s WITH TEMPLATE = template0 "
+ "OID = %u STRATEGY = FILE_COPY",
qdatname, dbCatId.oid);
}
else
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index af370768b6..45f095a230 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -534,9 +534,21 @@ static void
create_new_objects(void)
{
int dbnum;
+ PGconn *conn_new_template1;
prep_status_progress("Restoring database schemas in the new cluster");
+ /*
+ * Ensure that any changes to template0 are fully written out to disk
+ * prior to restoring the databases. This is necessary because we use the
+ * FILE_COPY strategy to create the databases (which testing has shown to
+ * be faster), and we skip the checkpoints that this strategy ordinarily
+ * incurs.
+ */
+ conn_new_template1 = connectToServer(&new_cluster, "template1");
+ PQclear(executeQueryOrDie(conn_new_template1, "CHECKPOINT"));
+ PQfinish(conn_new_template1);
+
/*
* We cannot process the template1 database concurrently with others,
* because when it's transiently dropped, connection attempts would fail.
--
2.39.3 (Apple Git-146)
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-19 13:07 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-19 13:07 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Fri, 14 Jun 2024 at 23:29, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Wed, Jun 12, 2024 at 09:41:01AM -0500, Nathan Bossart wrote:
> > Actually, I think you are right that we need a manual checkpoint, except I
> > think we need it to be after prepare_new_globals(). set_frozenxids()
> > connects to each database (including template0) and updates a bunch of
> > pg_class rows, and we probably want those on disk before we start copying
> > the files to create all the user's databases.
Good catch, I hadn't thought of that.
> Here is an updated patch.
LGTM.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-19 13:17 Robert Haas <robertmhaas@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 2 replies; 21+ messages in thread
From: Robert Haas @ 2024-06-19 13:17 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Fri, Jun 14, 2024 at 5:29 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> On Wed, Jun 12, 2024 at 09:41:01AM -0500, Nathan Bossart wrote:
> > Actually, I think you are right that we need a manual checkpoint, except I
> > think we need it to be after prepare_new_globals(). set_frozenxids()
> > connects to each database (including template0) and updates a bunch of
> > pg_class rows, and we probably want those on disk before we start copying
> > the files to create all the user's databases.
>
> Here is an updated patch.
OK, I have a (probably) stupid question. The comment says:
+ * In binary upgrade mode, we can skip this checkpoint because neither of
+ * these problems applies: we don't ever replay the WAL generated during
+ * pg_upgrade, and we don't concurrently modify template0 (not to mention
+ * that trying to take a backup during pg_upgrade is pointless).
But what happens if the system crashes during pg_upgrade? Does this
patch make things worse than they are today? And should we care?
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-19 13:37 Nathan Bossart <nathandbossart@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 21+ messages in thread
From: Nathan Bossart @ 2024-06-19 13:37 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Wed, Jun 19, 2024 at 09:17:00AM -0400, Robert Haas wrote:
> OK, I have a (probably) stupid question. The comment says:
>
> + * In binary upgrade mode, we can skip this checkpoint because neither of
> + * these problems applies: we don't ever replay the WAL generated during
> + * pg_upgrade, and we don't concurrently modify template0 (not to mention
> + * that trying to take a backup during pg_upgrade is pointless).
>
> But what happens if the system crashes during pg_upgrade? Does this
> patch make things worse than they are today? And should we care?
My understanding is that you basically have to restart the upgrade from
scratch if that happens. I suppose there could be a problem if you try to
use the half-upgraded cluster after a crash, but I imagine you have a good
chance of encountering other problems if you do that, too. So I don't
think we care...
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-19 13:38 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 0 replies; 21+ messages in thread
From: Matthias van de Meent @ 2024-06-19 13:38 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Nathan Bossart <nathandbossart@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Wed, 19 Jun 2024 at 15:17, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Jun 14, 2024 at 5:29 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> > On Wed, Jun 12, 2024 at 09:41:01AM -0500, Nathan Bossart wrote:
> > > Actually, I think you are right that we need a manual checkpoint, except I
> > > think we need it to be after prepare_new_globals(). set_frozenxids()
> > > connects to each database (including template0) and updates a bunch of
> > > pg_class rows, and we probably want those on disk before we start copying
> > > the files to create all the user's databases.
> >
> > Here is an updated patch.
>
> OK, I have a (probably) stupid question. The comment says:
>
> + * In binary upgrade mode, we can skip this checkpoint because neither of
> + * these problems applies: we don't ever replay the WAL generated during
> + * pg_upgrade, and we don't concurrently modify template0 (not to mention
> + * that trying to take a backup during pg_upgrade is pointless).
>
> But what happens if the system crashes during pg_upgrade? Does this
> patch make things worse than they are today? And should we care?
As Nathan just said, AFAIK we don't have a way to resume progress from
a crashed pg_upgrade, which implies you currently have to start over
with a fresh dbinit.
This patch wouldn't change that for the worse, it'd only add one more
process that depends on that behaviour.
Maybe in the future that'll change if pg_upgrade and pg_dump are made
smart enough to resume their restore progress across forceful
disconnects, but for now this patch seems like a nice performance
improvement.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-06-20 04:29 Michael Paquier <michael@paquier.xyz>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: Michael Paquier @ 2024-06-20 04:29 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
On Wed, Jun 19, 2024 at 08:37:17AM -0500, Nathan Bossart wrote:
> My understanding is that you basically have to restart the upgrade from
> scratch if that happens. I suppose there could be a problem if you try to
> use the half-upgraded cluster after a crash, but I imagine you have a good
> chance of encountering other problems if you do that, too. So I don't
> think we care...
It's never been assumed that it would be safe to redo a
pg_upgradeafter a crash on a cluster initdb'd for the upgrade, so I
don't think we need to care about that, as well.
One failure I suspect would quickly be faced is OIDs getting reused
again as these are currently kept consistent.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 21+ messages in thread
* Re: use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade
@ 2024-07-08 21:22 Nathan Bossart <nathandbossart@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Bossart @ 2024-07-08 21:22 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Dilip Kumar <dilipbalaut@gmail.com>; Ranier Vilela <ranier.vf@gmail.com>; pgsql-hackers
Committed.
--
nathan
^ permalink raw reply [nested|flat] 21+ messages in thread
end of thread, other threads:[~2024-07-08 21:22 UTC | newest]
Thread overview: 21+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-06-04 19:39 use CREATE DATABASE STRATEGY = FILE_COPY in pg_upgrade Nathan Bossart <nathandbossart@gmail.com>
2024-06-05 16:47 ` Ranier Vilela <ranier.vf@gmail.com>
2024-06-05 17:07 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-05 17:28 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-06-05 17:45 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-07 05:18 ` Dilip Kumar <dilipbalaut@gmail.com>
2024-06-07 06:27 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-06-07 08:27 ` Dilip Kumar <dilipbalaut@gmail.com>
2024-06-07 09:10 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-06-07 09:17 ` Dilip Kumar <dilipbalaut@gmail.com>
2024-06-11 02:01 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-11 08:39 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-06-11 16:03 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-12 14:41 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-14 21:29 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-19 13:07 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-06-19 13:17 ` Robert Haas <robertmhaas@gmail.com>
2024-06-19 13:37 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-20 04:29 ` Michael Paquier <michael@paquier.xyz>
2024-07-08 21:22 ` Nathan Bossart <nathandbossart@gmail.com>
2024-06-19 13:38 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox