public inbox for [email protected]
help / color / mirror / Atom feedRe: Option to not use ringbuffer in VACUUM, using it in failsafe mode
8+ messages / 5 participants
[nested] [flat]
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
@ 2023-04-06 21:44 Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Melanie Plageman @ 2023-04-06 21:44 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>
On Fri, Apr 07, 2023 at 09:12:32AM +1200, David Rowley wrote:
> On Fri, 7 Apr 2023 at 05:20, Melanie Plageman <[email protected]> wrote:
> > GUC name mentioned in comment is inconsistent with current GUC name.
> >
> > > +/*
> > > + * Upper and lower hard limits for the buffer access strategy ring size
> > > + * specified by the vacuum_buffer_usage_limit GUC and BUFFER_USAGE_LIMIT
> > > + * option to VACUUM and ANALYZE.
>
> I did adjust this. I wasn't sure it was incorrect as I mentioned "GUC"
> as in, the user facing setting.
Oh, actually maybe you are right then.
> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
> index bcc49aec45..220f9ee84c 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -2001,6 +2001,35 @@ include_dir 'conf.d'
> </listitem>
> </varlistentry>
>
> + <varlistentry id="guc-vacuum-buffer-usage-limit" xreflabel="vacuum_buffer_usage_limit">
> + <term>
> + <varname>vacuum_buffer_usage_limit</varname> (<type>integer</type>)
> + <indexterm>
> + <primary><varname>vacuum_buffer_usage_limit</varname> configuration parameter</primary>
> + </indexterm>
> + </term>
> + <listitem>
> + <para>
> + Specifies the size of <varname>shared_buffers</varname> to be reused
> + for each backend participating in a given invocation of
> + <command>VACUUM</command> or <command>ANALYZE</command> or in
> + autovacuum.
Rereading this, I think it is not a good sentence (my fault).
Perhaps we should use the same language as the BUFFER_USAGE_LIMIT
options. Something like:
Specifies the
<glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
ring buffer size used by each backend participating in a given
invocation of <command>VACUUM</command> or <command>ANALYZE</command> or
in autovacuum.
Last part is admittedly a bit awkward...
> diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
> index ea1d8960f4..995b4bd54a 100644
> --- a/src/backend/commands/vacuum.c
> +++ b/src/backend/commands/vacuum.c
> @@ -56,6 +56,7 @@
> #include "utils/acl.h"
> #include "utils/fmgroids.h"
> #include "utils/guc.h"
> +#include "utils/guc_hooks.h"
> #include "utils/memutils.h"
> #include "utils/pg_rusage.h"
> #include "utils/snapmgr.h"
> @@ -95,6 +96,26 @@ static VacOptValue get_vacoptval_from_boolean(DefElem *def);
> static bool vac_tid_reaped(ItemPointer itemptr, void *state);
> static int vac_cmp_itemptr(const void *left, const void *right);
>
> +/*
> + * GUC check function to ensure GUC value specified is within the allowable
> + * range.
> + */
> +bool
> +check_vacuum_buffer_usage_limit(int *newval, void **extra,
> + GucSource source)
> +{
I'm not so hot on this comment. It seems very...generic. Like it could
be the comment on any GUC check function. I'm also okay with leaving it
as is.
> @@ -341,7 +422,19 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
>
> MemoryContext old_context = MemoryContextSwitchTo(vac_context);
>
> - bstrategy = GetAccessStrategy(BAS_VACUUM);
> + Assert(ring_size >= -1);
> +
> + /*
> + * If BUFFER_USAGE_LIMIT was specified by the VACUUM or ANALYZE
> + * command, it overrides the value of VacuumBufferUsageLimit. Either
> + * value may be 0, in which case GetAccessStrategyWithSize() will
> + * return NULL, effectively allowing full use of shared buffers.
Maybe "unlimited" is better than "full"?
> + */
> + if (ring_size != -1)
> + bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, ring_size);
> + else
> + bstrategy = GetAccessStrategyWithSize(BAS_VACUUM, VacuumBufferUsageLimit);
> +
> MemoryContextSwitchTo(old_context);
> }
>
> diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
> @@ -365,6 +371,9 @@ parallel_vacuum_init(Relation rel, Relation *indrels, int nindexes,
> maintenance_work_mem / Min(parallel_workers, nindexes_mwm) :
> maintenance_work_mem;
>
> + /* Use the same buffer size for all workers */
I would say ring buffer size -- this sounds like it is the size of a
single buffer.
> + shared->ring_nbuffers = GetAccessStrategyBufferCount(bstrategy);
> +
> pg_atomic_init_u32(&(shared->cost_balance), 0);
> pg_atomic_init_u32(&(shared->active_nworkers), 0);
> pg_atomic_init_u32(&(shared->idx), 0);
> + * Upper and lower hard limits for the buffer access strategy ring size
> + * specified by the VacuumBufferUsageLimit GUC and BUFFER_USAGE_LIMIT option
I agree with your original usage of the actual GUC name, now that I
realize why you were doing it and am rereading it.
> + * to VACUUM and ANALYZE.
> + */
> +#define MIN_BAS_VAC_RING_SIZE_KB 128
> +#define MAX_BAS_VAC_RING_SIZE_KB (16 * 1024 * 1024)
Otherwise, LGTM.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
@ 2023-04-07 00:52 ` David Rowley <[email protected]>
2023-04-14 07:20 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: David Rowley @ 2023-04-07 00:52 UTC (permalink / raw)
To: David Rowley <[email protected]>; Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>
On Fri, 7 Apr 2023 at 09:44, Melanie Plageman <[email protected]> wrote:
> Otherwise, LGTM.
Thanks for looking. I've also taken Justin's comments about the
README into account and fixed that part.
I've pushed the patch after a little more adjusting. I added some
text to the docs that mention larger VACUUM_BUFFER_LIMITs can speed up
vacuum and also a reason why they might not want to go nuts with it.
I've also just now pushed the vacuumdb patch too. I ended up adjusting
some of the ERROR messages in the main patch after the following not
so nice user experience:
$ vacuumdb --buffer-usage-limit=1TB --analyze postgres
vacuumdb: vacuuming database "postgres"
SQL: VACUUM (SKIP_DATABASE_STATS, ANALYZE, BUFFER_USAGE_LIMIT '1TB')
vacuumdb: error: processing of database "postgres" failed: ERROR:
buffer_usage_limit option must be 0 or between 128 KB and 16777216 KB
$ vacuumdb --buffer-usage-limit=128KB --analyze postgres
vacuumdb: vacuuming database "postgres"
SQL: VACUUM (SKIP_DATABASE_STATS, ANALYZE, BUFFER_USAGE_LIMIT '128KB')
vacuumdb: error: processing of database "postgres" failed: ERROR:
value: "128KB": is invalid for buffer_usage_limit
HINT: Valid units for this parameter are "B", "kB", "MB", "GB", and "TB".
David
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
@ 2023-04-14 07:20 ` Peter Eisentraut <[email protected]>
2023-04-15 00:59 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Peter Eisentraut @ 2023-04-14 07:20 UTC (permalink / raw)
To: David Rowley <[email protected]>; Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>
On 07.04.23 02:52, David Rowley wrote:
> On Fri, 7 Apr 2023 at 09:44, Melanie Plageman <[email protected]> wrote:
>> Otherwise, LGTM.
>
> Thanks for looking. I've also taken Justin's comments about the
> README into account and fixed that part.
>
> I've pushed the patch after a little more adjusting. I added some
> text to the docs that mention larger VACUUM_BUFFER_LIMITs can speed up
> vacuum and also a reason why they might not want to go nuts with it.
I came across these new options and had a little bit of trouble figuring
them out from the documentation. Maybe this could be polished a bit.
vacuumdb --help says
--buffer-usage-limit=BUFSIZE
I can guess what a "SIZE" might be, but is "BUFSIZE" different from a
"SIZE"? Maybe simplify here.
On the vacuumdb man page, the placeholder is
<replaceable class="parameter">buffer_usage_limit</replaceable>
which is yet another way of phrasing it. Maybe also use "size" here?
The VACUUM man page says
BUFFER_USAGE_LIMIT [ <replaceable ...>string</replaceable> ]
which had me really confused. The detailed description later doesn't
give any further explanation of possible values, except that
<literal>0</literal> is apparently a possible value, which in my mind is
not a string. Then there is a link to guc-vacuum-buffer-usage-limit,
which lifts the mystery that this is really just an integer setting with
possible memory-size units, but it was really hard to figure that out
from the start!
Moreover, on the VACUUM man page, right below BUFFER_USAGE_LIMIT, it
explains the different kinds of accepted values, and "string" wasn't
added there. Maybe also change this to "size" here and add an
explanation there what kinds of sizes are possible.
Finally, the locations of the new options in the various documentation
places seems a bit random. The vacuumdb --help output and the man page
appear to be mostly alphabetical, so --buffer-usage-limit should be
after -a/--all. (Also note that right now the option isn't even in the
same place in the --help output versus the man page.)
The order of the options on the VACUUM man page doesn't make any sense
anymore. This isn't really the fault of this patch, but maybe it's time
to do a fresh reordering there.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-14 07:20 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Peter Eisentraut <[email protected]>
@ 2023-04-15 00:59 ` David Rowley <[email protected]>
2023-04-16 00:09 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: David Rowley @ 2023-04-15 00:59 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>
On Fri, 14 Apr 2023 at 19:20, Peter Eisentraut
<[email protected]> wrote:
>
> I came across these new options and had a little bit of trouble figuring
> them out from the documentation. Maybe this could be polished a bit.
>
> vacuumdb --help says
>
> --buffer-usage-limit=BUFSIZE
>
> I can guess what a "SIZE" might be, but is "BUFSIZE" different from a
> "SIZE"? Maybe simplify here.
>
> On the vacuumdb man page, the placeholder is
>
> <replaceable class="parameter">buffer_usage_limit</replaceable>
>
> which is yet another way of phrasing it. Maybe also use "size" here?
>
> The VACUUM man page says
>
> BUFFER_USAGE_LIMIT [ <replaceable ...>string</replaceable> ]
>
> which had me really confused. The detailed description later doesn't
> give any further explanation of possible values, except that
> <literal>0</literal> is apparently a possible value, which in my mind is
> not a string. Then there is a link to guc-vacuum-buffer-usage-limit,
> which lifts the mystery that this is really just an integer setting with
> possible memory-size units, but it was really hard to figure that out
> from the start!
>
> Moreover, on the VACUUM man page, right below BUFFER_USAGE_LIMIT, it
> explains the different kinds of accepted values, and "string" wasn't
> added there. Maybe also change this to "size" here and add an
> explanation there what kinds of sizes are possible.
>
> Finally, the locations of the new options in the various documentation
> places seems a bit random. The vacuumdb --help output and the man page
> appear to be mostly alphabetical, so --buffer-usage-limit should be
> after -a/--all. (Also note that right now the option isn't even in the
> same place in the --help output versus the man page.)
These are all valid points. I've attached a patch aiming to address
each of them.
> The order of the options on the VACUUM man page doesn't make any sense
> anymore. This isn't really the fault of this patch, but maybe it's time
> to do a fresh reordering there.
Agreed, that likely wasn't a big problem say about 5 years ago when we
had far fewer options, but the number has grown quite a bit since
then.
Right after I fix the points you've mentioned seems a good time to address that.
David
Attachments:
[application/octet-stream] fix_buffer_usage_limit_docs.patch (7.3K, ../../CAApHDvrYafdxurB+TuCs0+kZXphsHa+VBBSd3QaE2eHycXL3vw@mail.gmail.com/2-fix_buffer_usage_limit_docs.patch)
download | inline diff:
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 94e30f1ce7..6f0bb4631b 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -28,7 +28,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
- BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
+ BUFFER_USAGE_LIMIT [ <replaceable class="parameter">size</replaceable> ]
<phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
@@ -110,7 +110,8 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
<xref linkend="guc-vacuum-buffer-usage-limit"/>. Higher settings can
allow <command>ANALYZE</command> to run more quickly, but having too
large a setting may cause too many other useful pages to be evicted from
- shared buffers.
+ shared buffers. The minimum value is <literal>128 kB</literal> and the
+ maximum value is <literal>16 GB</literal>.
</para>
</listitem>
</varlistentry>
@@ -129,6 +130,19 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">size</replaceable></term>
+ <listitem>
+ <para>
+ Specifies an amount of memory in kilobytes. Sizes may also be specified
+ as a string containing the size followed by any one of the following
+ memory units: <literal>kB</literal> (kilobytes), <literal>MB</literal>
+ (megabytes), <literal>GB</literal> (gigabytes), or
+ <literal>TB</literal> (terabytes).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">table_name</replaceable></term>
<listitem>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index dd0fbb8cb7..54f47a0c64 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -39,7 +39,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
PARALLEL <replaceable class="parameter">integer</replaceable>
SKIP_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
ONLY_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
- BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
+ BUFFER_USAGE_LIMIT [ <replaceable class="parameter">size</replaceable> ]
<phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
@@ -364,7 +364,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
<xref linkend="guc-vacuum-buffer-usage-limit"/>. Higher settings can
allow <command>VACUUM</command> to run more quickly, but having too
large a setting may cause too many other useful pages to be evicted from
- shared buffers.
+ shared buffers. The minimum value is <literal>128 kB</literal> and the
+ maximum value is <literal>16 GB</literal>.
</para>
</listitem>
</varlistentry>
@@ -392,6 +393,19 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">size</replaceable></term>
+ <listitem>
+ <para>
+ Specifies an amount of memory in kilobytes. Sizes may also be specified
+ as a string containing the size followed by any one of the following
+ memory units: <literal>kB</literal> (kilobytes), <literal>MB</literal>
+ (megabytes), <literal>GB</literal> (gigabytes), or
+ <literal>TB</literal> (terabytes).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">table_name</replaceable></term>
<listitem>
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 8280cf0fb0..caf97af215 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -121,6 +121,19 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--buffer-usage-limit <replaceable class="parameter">size</replaceable></option></term>
+ <listitem>
+ <para>
+ Specifies the
+ <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
+ ring buffer size for a given invocation of <application>vacuumdb</application>.
+ This size is used to calculate the number of shared buffers which will
+ be reused as part of this strategy. See <xref linkend="sql-vacuum"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option><optional>-d</optional> <replaceable class="parameter">dbname</replaceable></option></term>
<term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
@@ -278,19 +291,6 @@ PostgreSQL documentation
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>--buffer-usage-limit <replaceable class="parameter">buffer_usage_limit</replaceable></option></term>
- <listitem>
- <para>
- Specifies the
- <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
- ring buffer size for a given invocation of <application>vacuumdb</application>.
- This size is used to calculate the number of shared buffers which will
- be reused as part of this strategy. See <xref linkend="sql-vacuum"/>.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><option>-n <replaceable class="parameter">schema</replaceable></option></term>
<term><option>--schema=<replaceable class="parameter">schema</replaceable></option></term>
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 1241644ed5..687af9c1f3 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -1128,6 +1128,7 @@ help(const char *progname)
printf(_(" %s [OPTION]... [DBNAME]\n"), progname);
printf(_("\nOptions:\n"));
printf(_(" -a, --all vacuum all databases\n"));
+ printf(_(" --buffer-usage-limit=SIZE size of ring buffer used for vacuum\n"));
printf(_(" -d, --dbname=DBNAME database to vacuum\n"));
printf(_(" --disable-page-skipping disable all page-skipping behavior\n"));
printf(_(" -e, --echo show the commands being sent to the server\n"));
@@ -1136,7 +1137,6 @@ help(const char *progname)
printf(_(" --force-index-cleanup always remove index entries that point to dead tuples\n"));
printf(_(" -j, --jobs=NUM use this many concurrent connections to vacuum\n"));
printf(_(" --min-mxid-age=MXID_AGE minimum multixact ID age of tables to vacuum\n"));
- printf(_(" --buffer-usage-limit=BUFSIZE size of ring buffer used for vacuum\n"));
printf(_(" --min-xid-age=XID_AGE minimum transaction ID age of tables to vacuum\n"));
printf(_(" --no-index-cleanup don't remove index entries that point to dead tuples\n"));
printf(_(" --no-process-main skip the main relation\n"));
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-14 07:20 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Peter Eisentraut <[email protected]>
2023-04-15 00:59 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
@ 2023-04-16 00:09 ` David Rowley <[email protected]>
2023-04-26 08:47 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: David Rowley @ 2023-04-16 00:09 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>
On Sat, 15 Apr 2023 at 12:59, David Rowley <[email protected]> wrote:
> These are all valid points. I've attached a patch aiming to address
> each of them.
I tweaked this a little further and pushed it.
David
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-14 07:20 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Peter Eisentraut <[email protected]>
2023-04-15 00:59 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-16 00:09 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
@ 2023-04-26 08:47 ` Masahiko Sawada <[email protected]>
2023-04-26 11:26 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Masahiko Sawada @ 2023-04-26 08:47 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>
Hi,
On Sun, Apr 16, 2023 at 9:09 AM David Rowley <[email protected]> wrote:
>
> On Sat, 15 Apr 2023 at 12:59, David Rowley <[email protected]> wrote:
> > These are all valid points. I've attached a patch aiming to address
> > each of them.
>
> I tweaked this a little further and pushed it.
>
I realized that the value of vacuum_buffer_usage_limit parameter in
postgresql.conf.sample doesn't have the unit:
#vacuum_buffer_usage_limit = 256 # size of vacuum and analyze buffer
access strategy ring.
# 0 to disable vacuum buffer access strategy
# range 128kB to 16GB
It works but I think we might want to add the unit kB for
understandability and consistency with other GUC_UNIT_KB parameters.
I've attached a small patch that adds the unit and aligns the indent
of the comments to the perimeter parameters.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] add_unit_to_vacuum_buffer_usage_limit.patch (738B, ../../CAD21AoCBSqmqOKVH4Q256DeCC_UE50gu1sgixcjLFZGLEbABVA@mail.gmail.com/2-add_unit_to_vacuum_buffer_usage_limit.patch)
download | inline diff:
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 0609853995..b70c66ca87 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -158,9 +158,9 @@
# mmap
# (change requires restart)
#min_dynamic_shared_memory = 0MB # (change requires restart)
-#vacuum_buffer_usage_limit = 256 # size of vacuum and analyze buffer access strategy ring.
- # 0 to disable vacuum buffer access strategy
- # range 128kB to 16GB
+#vacuum_buffer_usage_limit = 256kB # size of vacuum and analyze buffer access strategy ring.
+ # 0 to disable vacuum buffer access strategy
+ # range 128kB to 16GB
# - Disk -
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-14 07:20 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Peter Eisentraut <[email protected]>
2023-04-15 00:59 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-16 00:09 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode David Rowley <[email protected]>
2023-04-26 08:47 ` Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Masahiko Sawada <[email protected]>
@ 2023-04-26 11:26 ` David Rowley <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: David Rowley @ 2023-04-26 11:26 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; Justin Pryzby <[email protected]>; Bharath Rupireddy <[email protected]>; Tom Lane <[email protected]>; Peter Geoghegan <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>
On Wed, 26 Apr 2023, 8:48 pm Masahiko Sawada, <[email protected]> wrote:
> I realized that the value of vacuum_buffer_usage_limit parameter in
> postgresql.conf.sample doesn't have the unit:
>
> #vacuum_buffer_usage_limit = 256 # size of vacuum and analyze buffer
> access strategy ring.
> # 0 to disable vacuum buffer access
> strategy
> # range 128kB to 16GB
>
> It works but I think we might want to add the unit kB for
> understandability and consistency with other GUC_UNIT_KB parameters.
> I've attached a small patch that adds the unit and aligns the indent
> of the comments to the perimeter parameters.
>
I'm not currently able to check, but if work_mem has a unit in the sample
conf then I agree that vacuum_buffer_usage_limit should too.
I'm fine for you to go ahead and adjust this, otherwise it'll be Monday
before I can.
David
>
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v4 2/4] pg_upgrade: bump minimum supported version to v10
@ 2026-04-17 18:20 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Nathan Bossart @ 2026-04-17 18:20 UTC (permalink / raw)
---
doc/src/sgml/ref/pgupgrade.sgml | 2 +-
src/bin/pg_upgrade/check.c | 157 +----------------------
src/bin/pg_upgrade/controldata.c | 12 +-
src/bin/pg_upgrade/exec.c | 23 ----
src/bin/pg_upgrade/file.c | 164 -------------------------
src/bin/pg_upgrade/multixact_rewrite.c | 11 +-
src/bin/pg_upgrade/pg_upgrade.c | 31 +----
src/bin/pg_upgrade/pg_upgrade.h | 29 -----
src/bin/pg_upgrade/relfilenumber.c | 37 +-----
src/bin/pg_upgrade/version.c | 127 -------------------
10 files changed, 16 insertions(+), 577 deletions(-)
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 38ca09b423c..c4ed75211db 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -67,7 +67,7 @@ PostgreSQL documentation
</para>
<para>
- <application>pg_upgrade</application> supports upgrades from 9.2.X and later to the current
+ <application>pg_upgrade</application> supports upgrades from 10.X and later to the current
major release of <productname>PostgreSQL</productname>, including snapshot and beta releases.
</para>
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f5c93e611d2..f1e35a6af47 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -28,7 +28,6 @@ static void check_for_incompatible_polymorphics(ClusterInfo *cluster);
static void check_for_tables_with_oids(ClusterInfo *cluster);
static void check_for_not_null_inheritance(ClusterInfo *cluster);
static void check_for_gist_inet_ops(ClusterInfo *cluster);
-static void check_for_pg_role_prefix(ClusterInfo *cluster);
static void check_for_new_tablespace_dir(void);
static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
static void check_for_unicode_update(ClusterInfo *cluster);
@@ -128,26 +127,6 @@ static DataTypesUsageChecks data_types_usage_checks[] =
.threshold_version = ALL_VERSIONS
},
- /*
- * 9.3 -> 9.4 Fully implement the 'line' data type in 9.4, which
- * previously returned "not enabled" by default and was only functionally
- * enabled with a compile-time switch; as of 9.4 "line" has a different
- * on-disk representation format.
- */
- {
- .status = gettext_noop("Checking for incompatible \"line\" data type"),
- .report_filename = "tables_using_line.txt",
- .base_query =
- "SELECT 'pg_catalog.line'::pg_catalog.regtype AS oid",
- .report_text =
- gettext_noop("Your installation contains the \"line\" data type in user tables.\n"
- "This data type changed its internal and input/output format\n"
- "between your old and new versions so this\n"
- "cluster cannot currently be upgraded. You can\n"
- "drop the problem columns and restart the upgrade.\n"),
- .threshold_version = 903
- },
-
/*
* pg_upgrade only preserves these system values: pg_class.oid pg_type.oid
* pg_enum.oid
@@ -209,30 +188,6 @@ static DataTypesUsageChecks data_types_usage_checks[] =
.threshold_version = 1500
},
- /*
- * It's no longer allowed to create tables or views with "unknown"-type
- * columns. We do not complain about views with such columns, because
- * they should get silently converted to "text" columns during the DDL
- * dump and reload; it seems unlikely to be worth making users do that by
- * hand. However, if there's a table with such a column, the DDL reload
- * will fail, so we should pre-detect that rather than failing
- * mid-upgrade. Worse, if there's a matview with such a column, the DDL
- * reload will silently change it to "text" which won't match the on-disk
- * storage (which is like "cstring"). So we *must* reject that.
- */
- {
- .status = gettext_noop("Checking for invalid \"unknown\" user columns"),
- .report_filename = "tables_using_unknown.txt",
- .base_query =
- "SELECT 'pg_catalog.unknown'::pg_catalog.regtype AS oid",
- .report_text =
- gettext_noop("Your installation contains the \"unknown\" data type in user tables.\n"
- "This data type is no longer allowed in tables, so this cluster\n"
- "cannot currently be upgraded. You can drop the problem columns\n"
- "and restart the upgrade.\n"),
- .threshold_version = 906
- },
-
/*
* PG 12 changed the 'sql_identifier' type storage to be based on name,
* not varchar, which breaks on-disk format for existing data. So we need
@@ -255,23 +210,6 @@ static DataTypesUsageChecks data_types_usage_checks[] =
.threshold_version = 1100
},
- /*
- * JSONB changed its storage format during 9.4 beta, so check for it.
- */
- {
- .status = gettext_noop("Checking for incompatible \"jsonb\" data type in user tables"),
- .report_filename = "tables_using_jsonb.txt",
- .base_query =
- "SELECT 'pg_catalog.jsonb'::pg_catalog.regtype AS oid",
- .report_text =
- gettext_noop("Your installation contains the \"jsonb\" data type in user tables.\n"
- "The internal format of \"jsonb\" changed during 9.4 beta so this\n"
- "cluster cannot currently be upgraded. You can drop the problem \n"
- "columns and restart the upgrade.\n"),
- .threshold_version = MANUAL_CHECK,
- .version_hook = jsonb_9_4_check_applicable
- },
-
/*
* PG 12 removed types abstime, reltime, tinterval.
*/
@@ -712,20 +650,6 @@ check_and_dump_old_cluster(void)
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1800)
check_for_gist_inet_ops(&old_cluster);
- /*
- * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged
- * hash indexes
- */
- if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
- {
- if (user_opts.check)
- old_9_6_invalidate_hash_indexes(&old_cluster, true);
- }
-
- /* 9.5 and below should not have roles starting with pg_ */
- if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905)
- check_for_pg_role_prefix(&old_cluster);
-
/*
* While not a check option, we do this now because this is the only time
* the old server is running.
@@ -772,20 +696,6 @@ check_new_cluster(void)
* system boundaries.
*/
check_hard_link(TRANSFER_MODE_SWAP);
-
- /*
- * There are a few known issues with using --swap to upgrade from
- * versions older than 10. For example, the sequence tuple format
- * changed in v10, and the visibility map format changed in 9.6.
- * While such problems are not insurmountable (and we may have to
- * deal with similar problems in the future, anyway), it doesn't
- * seem worth the effort to support swap mode for upgrades from
- * long-unsupported versions.
- */
- if (GET_MAJOR_VERSION(old_cluster.major_version) < 1000)
- pg_fatal("Swap mode can only upgrade clusters from PostgreSQL version %s and later.",
- "10");
-
break;
}
@@ -831,10 +741,6 @@ issue_warnings_and_set_wal_level(void)
*/
start_postmaster(&new_cluster, true);
- /* Reindex hash indexes for old < 10.0 */
- if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906)
- old_9_6_invalidate_hash_indexes(&new_cluster, false);
-
report_extension_updates(&new_cluster);
stop_postmaster(false);
@@ -892,9 +798,9 @@ check_cluster_versions(void)
* upgrades
*/
- if (GET_MAJOR_VERSION(old_cluster.major_version) < 902)
+ if (GET_MAJOR_VERSION(old_cluster.major_version) < 10)
pg_fatal("This utility can only upgrade from PostgreSQL version %s and later.",
- "9.2");
+ "10");
/* Only current PG version is supported as a target */
if (GET_MAJOR_VERSION(new_cluster.major_version) != GET_MAJOR_VERSION(PG_VERSION_NUM))
@@ -1569,12 +1475,10 @@ check_for_incompatible_polymorphics(ClusterInfo *cluster)
", 'array_cat(anyarray,anyarray)'"
", 'array_prepend(anyelement,anyarray)'");
- if (GET_MAJOR_VERSION(cluster->major_version) >= 903)
appendPQExpBufferStr(&old_polymorphics,
", 'array_remove(anyarray,anyelement)'"
", 'array_replace(anyarray,anyelement,anyelement)'");
- if (GET_MAJOR_VERSION(cluster->major_version) >= 905)
appendPQExpBufferStr(&old_polymorphics,
", 'array_position(anyarray,anyelement)'"
", 'array_position(anyarray,anyelement,integer)'"
@@ -1870,63 +1774,6 @@ check_for_gist_inet_ops(ClusterInfo *cluster)
check_ok();
}
-/*
- * check_for_pg_role_prefix()
- *
- * Versions older than 9.6 should not have any pg_* roles
- */
-static void
-check_for_pg_role_prefix(ClusterInfo *cluster)
-{
- PGresult *res;
- PGconn *conn = connectToServer(cluster, "template1");
- int ntups;
- int i_roloid;
- int i_rolname;
- FILE *script = NULL;
- char output_path[MAXPGPATH];
-
- prep_status("Checking for roles starting with \"pg_\"");
-
- snprintf(output_path, sizeof(output_path), "%s/%s",
- log_opts.basedir,
- "pg_role_prefix.txt");
-
- res = executeQueryOrDie(conn,
- "SELECT oid AS roloid, rolname "
- "FROM pg_catalog.pg_roles "
- "WHERE rolname ~ '^pg_'");
-
- ntups = PQntuples(res);
- i_roloid = PQfnumber(res, "roloid");
- i_rolname = PQfnumber(res, "rolname");
- for (int rowno = 0; rowno < ntups; rowno++)
- {
- if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
- pg_fatal("could not open file \"%s\": %m", output_path);
- fprintf(script, "%s (oid=%s)\n",
- PQgetvalue(res, rowno, i_rolname),
- PQgetvalue(res, rowno, i_roloid));
- }
-
- PQclear(res);
-
- PQfinish(conn);
-
- if (script)
- {
- fclose(script);
- pg_log(PG_REPORT, "fatal");
- pg_fatal("Your installation contains roles starting with \"pg_\".\n"
- "\"pg_\" is a reserved prefix for system roles. The cluster\n"
- "cannot be upgraded until these roles are renamed.\n"
- "A list of roles starting with \"pg_\" is in the file:\n"
- " %s", output_path);
- }
- else
- check_ok();
-}
-
/*
* Callback function for processing results of query for
* check_for_user_defined_encoding_conversions()'s UpgradeTask. If the query
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index cffcd4b0eba..8188355240f 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -602,14 +602,12 @@ get_control_data(ClusterInfo *cluster)
/* verify that we got all the mandatory pg_control data */
if (!got_xid || !got_oid ||
!got_multi || !got_oldestxid ||
- (!got_oldestmulti &&
- cluster->controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER) ||
+ !got_oldestmulti ||
!got_mxoff || (!live_check && !got_nextxlogfile) ||
!got_float8_pass_by_value || !got_align || !got_blocksz ||
!got_largesz || !got_walsz || !got_walseg || !got_ident ||
!got_index || !got_toast ||
- (!got_large_object &&
- cluster->controldata.ctrl_ver >= LARGE_OBJECT_SIZE_PG_CONTROL_VER) ||
+ !got_large_object ||
!got_date_is_int || !got_data_checksum_version ||
(!got_default_char_signedness &&
cluster->controldata.cat_ver >= DEFAULT_CHAR_SIGNEDNESS_CAT_VER))
@@ -630,8 +628,7 @@ get_control_data(ClusterInfo *cluster)
if (!got_multi)
pg_log(PG_REPORT, " latest checkpoint next MultiXactId");
- if (!got_oldestmulti &&
- cluster->controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER)
+ if (!got_oldestmulti)
pg_log(PG_REPORT, " latest checkpoint oldest MultiXactId");
if (!got_oldestxid)
@@ -670,8 +667,7 @@ get_control_data(ClusterInfo *cluster)
if (!got_toast)
pg_log(PG_REPORT, " maximum TOAST chunk size");
- if (!got_large_object &&
- cluster->controldata.ctrl_ver >= LARGE_OBJECT_SIZE_PG_CONTROL_VER)
+ if (!got_large_object)
pg_log(PG_REPORT, " large-object chunk size");
if (!got_date_is_int)
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 563b4a6b5fa..44355feea30 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -55,16 +55,7 @@ get_bin_version(ClusterInfo *cluster)
if (sscanf(cmd_output, "%*s %*s %d.%d", &v1, &v2) < 1)
pg_fatal("could not get pg_ctl version output from %s", cmd);
- if (v1 < 10)
- {
- /* old style, e.g. 9.6.1 */
- cluster->bin_version = v1 * 10000 + v2 * 100;
- }
- else
- {
- /* new style, e.g. 10.1 */
cluster->bin_version = v1 * 10000;
- }
}
@@ -353,17 +344,7 @@ check_data_dir(ClusterInfo *cluster)
check_single_dir(pg_data, "pg_subtrans");
check_single_dir(pg_data, PG_TBLSPC_DIR);
check_single_dir(pg_data, "pg_twophase");
-
- /* pg_xlog has been renamed to pg_wal in v10 */
- if (GET_MAJOR_VERSION(cluster->major_version) <= 906)
- check_single_dir(pg_data, "pg_xlog");
- else
check_single_dir(pg_data, "pg_wal");
-
- /* pg_clog has been renamed to pg_xact in v10 */
- if (GET_MAJOR_VERSION(cluster->major_version) <= 906)
- check_single_dir(pg_data, "pg_clog");
- else
check_single_dir(pg_data, "pg_xact");
}
@@ -404,10 +385,6 @@ check_bin_dir(ClusterInfo *cluster, bool check_versions)
*/
get_bin_version(cluster);
- /* pg_resetxlog has been renamed to pg_resetwal in version 10 */
- if (GET_MAJOR_VERSION(cluster->bin_version) <= 906)
- check_exec(cluster->bindir, "pg_resetxlog", check_versions);
- else
check_exec(cluster->bindir, "pg_resetwal", check_versions);
if (cluster == &new_cluster)
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 5b276008614..af82c0de490 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -20,12 +20,8 @@
#include <linux/fs.h>
#endif
-#include "access/visibilitymapdefs.h"
#include "common/file_perm.h"
#include "pg_upgrade.h"
-#include "storage/bufpage.h"
-#include "storage/checksum.h"
-#include "storage/checksum_impl.h"
/*
@@ -196,166 +192,6 @@ linkFile(const char *src, const char *dst,
}
-/*
- * rewriteVisibilityMap()
- *
- * Transform a visibility map file, copying from src to dst.
- * schemaName/relName are relation's SQL name (used for error messages only).
- *
- * In versions of PostgreSQL prior to catversion 201603011, PostgreSQL's
- * visibility map included one bit per heap page; it now includes two.
- * When upgrading a cluster from before that time to a current PostgreSQL
- * version, we could refuse to copy visibility maps from the old cluster
- * to the new cluster; the next VACUUM would recreate them, but at the
- * price of scanning the entire table. So, instead, we rewrite the old
- * visibility maps in the new format. That way, the all-visible bits
- * remain set for the pages for which they were set previously. The
- * all-frozen bits are never set by this conversion; we leave that to VACUUM.
- */
-void
-rewriteVisibilityMap(const char *fromfile, const char *tofile,
- const char *schemaName, const char *relName)
-{
- int src_fd;
- int dst_fd;
- PGIOAlignedBlock buffer;
- PGIOAlignedBlock new_vmbuf;
- ssize_t totalBytesRead = 0;
- ssize_t src_filesize;
- int rewriteVmBytesPerPage;
- BlockNumber new_blkno = 0;
- struct stat statbuf;
-
- /* Compute number of old-format bytes per new page */
- rewriteVmBytesPerPage = (BLCKSZ - SizeOfPageHeaderData) / 2;
-
- if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0)
- pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %m",
- schemaName, relName, fromfile);
-
- if (fstat(src_fd, &statbuf) != 0)
- pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %m",
- schemaName, relName, fromfile);
-
- if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
- pg_file_create_mode)) < 0)
- pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %m",
- schemaName, relName, tofile);
-
- /* Save old file size */
- src_filesize = statbuf.st_size;
-
- /*
- * Turn each visibility map page into 2 pages one by one. Each new page
- * has the same page header as the old one. If the last section of the
- * last page is empty, we skip it, mostly to avoid turning one-page
- * visibility maps for small relations into two pages needlessly.
- */
- while (totalBytesRead < src_filesize)
- {
- ssize_t bytesRead;
- char *old_cur;
- char *old_break;
- char *old_blkend;
- PageHeaderData pageheader;
- bool old_lastblk;
-
- if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ)
- {
- if (bytesRead < 0)
- pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %m",
- schemaName, relName, fromfile);
- else
- pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"",
- schemaName, relName, fromfile);
- }
-
- totalBytesRead += BLCKSZ;
- old_lastblk = (totalBytesRead == src_filesize);
-
- /* Save the page header data */
- memcpy(&pageheader, buffer.data, SizeOfPageHeaderData);
-
- /*
- * These old_* variables point to old visibility map page. old_cur
- * points to current position on old page. old_blkend points to end of
- * old block. old_break is the end+1 position on the old page for the
- * data that will be transferred to the current new page.
- */
- old_cur = buffer.data + SizeOfPageHeaderData;
- old_blkend = buffer.data + bytesRead;
- old_break = old_cur + rewriteVmBytesPerPage;
-
- while (old_break <= old_blkend)
- {
- char *new_cur;
- bool empty = true;
- bool old_lastpart;
-
- /* First, copy old page header to new page */
- memcpy(new_vmbuf.data, &pageheader, SizeOfPageHeaderData);
-
- /* Rewriting the last part of the last old page? */
- old_lastpart = old_lastblk && (old_break == old_blkend);
-
- new_cur = new_vmbuf.data + SizeOfPageHeaderData;
-
- /* Process old page bytes one by one, and turn it into new page. */
- while (old_cur < old_break)
- {
- uint8 byte = *(uint8 *) old_cur;
- uint16 new_vmbits = 0;
- int i;
-
- /* Generate new format bits while keeping old information */
- for (i = 0; i < BITS_PER_BYTE; i++)
- {
- if (byte & (1 << i))
- {
- empty = false;
- new_vmbits |=
- VISIBILITYMAP_ALL_VISIBLE << (BITS_PER_HEAPBLOCK * i);
- }
- }
-
- /* Copy new visibility map bytes to new-format page */
- new_cur[0] = (char) (new_vmbits & 0xFF);
- new_cur[1] = (char) (new_vmbits >> 8);
-
- old_cur++;
- new_cur += BITS_PER_HEAPBLOCK;
- }
-
- /* If the last part of the last page is empty, skip writing it */
- if (old_lastpart && empty)
- break;
-
- /* Set new checksum for visibility map page, if enabled */
- if (new_cluster.controldata.data_checksum_version != PG_DATA_CHECKSUM_OFF)
- ((PageHeader) new_vmbuf.data)->pd_checksum =
- pg_checksum_page(new_vmbuf.data, new_blkno);
-
- errno = 0;
- if (write(dst_fd, new_vmbuf.data, BLCKSZ) != BLCKSZ)
- {
- /* if write didn't set errno, assume problem is no disk space */
- if (errno == 0)
- errno = ENOSPC;
- pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %m",
- schemaName, relName, tofile);
- }
-
- /* Advance for next new page */
- old_break += rewriteVmBytesPerPage;
- new_blkno++;
- }
- }
-
- /* Clean up */
- close(dst_fd);
- close(src_fd);
-}
-
void
check_file_clone(void)
{
diff --git a/src/bin/pg_upgrade/multixact_rewrite.c b/src/bin/pg_upgrade/multixact_rewrite.c
index 823984ec8f3..c45b3183684 100644
--- a/src/bin/pg_upgrade/multixact_rewrite.c
+++ b/src/bin/pg_upgrade/multixact_rewrite.c
@@ -25,10 +25,7 @@ static void RecordMultiXactMembers(SlruSegState *members_writer,
* 32-bit offsets to the current format.
*
* Multixids in the range [from_multi, to_multi) are read from the old
- * cluster, and written in the new format. An important edge case is that if
- * from_multi == to_multi, this initializes the new pg_multixact files in the
- * new format without trying to open any old files. (We rely on that when
- * upgrading from PostgreSQL version 9.2 or below.)
+ * cluster, and written in the new format.
*
* Returns the new nextOffset value; the caller should set it in the new
* control file. The new members always start from offset 1, regardless of
@@ -42,6 +39,7 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi)
SlruSegState *members_writer;
char dir[MAXPGPATH] = {0};
bool prev_multixid_valid = false;
+ OldMultiXactReader *old_reader;
/*
* The range of valid multi XIDs is unchanged by the conversion (they are
@@ -63,10 +61,6 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi)
* Convert old multixids, if needed, by reading them one-by-one from the
* old cluster.
*/
- if (to_multi != from_multi)
- {
- OldMultiXactReader *old_reader;
-
old_reader = AllocOldMultiXactRead(old_cluster.pgdata,
old_cluster.controldata.chkpnt_nxtmulti,
old_cluster.controldata.chkpnt_nxtmxoff);
@@ -113,7 +107,6 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi)
}
FreeOldMultiXactReader(old_reader);
- }
/* Write the final 'next' offset to the last SLRU page */
RecordMultiXactOffset(offsets_writer, to_multi,
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 2127d297bfe..e5d7920c1b1 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -714,13 +714,6 @@ create_new_objects(void)
end_progress_output();
check_ok();
- /*
- * We don't have minmxids for databases or relations in pre-9.3 clusters,
- * so set those after we have restored the schema.
- */
- if (GET_MAJOR_VERSION(old_cluster.major_version) <= 902)
- set_frozenxids(true);
-
/* update new_cluster info now that we have objects in the databases */
get_db_rel_and_slot_infos(&new_cluster);
}
@@ -777,10 +770,7 @@ copy_xact_xlog_xid(void)
* Copy old commit logs to new data dir. pg_clog has been renamed to
* pg_xact in post-10 clusters.
*/
- copy_subdir_files(GET_MAJOR_VERSION(old_cluster.major_version) <= 906 ?
- "pg_clog" : "pg_xact",
- GET_MAJOR_VERSION(new_cluster.major_version) <= 906 ?
- "pg_clog" : "pg_xact");
+ copy_subdir_files("pg_xact", "pg_xact");
prep_status("Setting oldest XID for new cluster");
exec_prog(UTILITY_LOG_FILE, NULL, true, true,
@@ -809,7 +799,6 @@ copy_xact_xlog_xid(void)
check_ok();
/* Copy or convert pg_multixact files */
- Assert(new_cluster.controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER);
Assert(new_cluster.controldata.cat_ver >= MULTIXACTOFFSET_FORMATCHANGE_CAT_VER);
if (old_cluster.controldata.cat_ver >= MULTIXACTOFFSET_FORMATCHANGE_CAT_VER)
{
@@ -844,25 +833,7 @@ copy_xact_xlog_xid(void)
* Determine the range of multixacts to convert.
*/
nxtmulti = old_cluster.controldata.chkpnt_nxtmulti;
- if (old_cluster.controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER)
- {
- /* Versions 9.3 - 18: convert all multixids */
oldstMulti = old_cluster.controldata.chkpnt_oldstMulti;
- }
- else
- {
- /*
- * In PostgreSQL 9.2 and below, multitransactions were only used
- * for row locking, and as such don't need to be preserved during
- * upgrade. In that case, we utilize rewrite_multixacts() just to
- * initialize new, empty files in the new format.
- *
- * It's important that the oldest multi is set to the latest value
- * used by the old system, so that multixact.c returns the empty
- * set for multis that might be present on disk.
- */
- oldstMulti = nxtmulti;
- }
/* handle wraparound */
if (nxtmulti < FirstMultiXactId)
nxtmulti = FirstMultiXactId;
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ccd1ac0d013..d6e5bca5792 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -101,19 +101,6 @@ extern char *output_files[];
#endif
-/*
- * The format of visibility map was changed with this 9.6 commit.
- */
-#define VISIBILITY_MAP_FROZEN_BIT_CAT_VER 201603011
-
-/*
- * pg_multixact format changed in 9.3 commit 0ac5ad5134f2769ccbaefec73844f85,
- * ("Improve concurrency of foreign key locking") which also updated catalog
- * version to this value. pg_upgrade behavior depends on whether old and new
- * server versions are both newer than this, or only the new one is.
- */
-#define MULTIXACT_FORMATCHANGE_CAT_VER 201301231
-
/*
* MultiXactOffset was changed from 32-bit to 64-bit in version 19, at this
* catalog version. pg_multixact files need to be converted when upgrading
@@ -121,17 +108,6 @@ extern char *output_files[];
*/
#define MULTIXACTOFFSET_FORMATCHANGE_CAT_VER 202512091
-/*
- * large object chunk size added to pg_controldata,
- * commit 5f93c37805e7485488480916b4585e098d3cc883
- */
-#define LARGE_OBJECT_SIZE_PG_CONTROL_VER 942
-
-/*
- * change in JSONB format during 9.4 beta
- */
-#define JSONB_FORMAT_CHANGE_CAT_VER 201409291
-
/*
* The control file was changed to have the default char signedness,
* commit 44fe30fdab6746a287163e7cc093fd36cda8eb92
@@ -429,8 +405,6 @@ void copyFileByRange(const char *src, const char *dst,
const char *schemaName, const char *relName);
void linkFile(const char *src, const char *dst,
const char *schemaName, const char *relName);
-void rewriteVisibilityMap(const char *fromfile, const char *tofile,
- const char *schemaName, const char *relName);
void check_file_clone(void);
void check_copy_file_range(void);
void check_hard_link(transferMode transfer_mode);
@@ -500,10 +474,7 @@ unsigned int str2uint(const char *str);
/* version.c */
-bool jsonb_9_4_check_applicable(ClusterInfo *cluster);
bool protocol_negotiation_supported(const ClusterInfo *cluster);
-void old_9_6_invalidate_hash_indexes(ClusterInfo *cluster,
- bool check_mode);
void report_extension_updates(ClusterInfo *cluster);
diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c
index d5088447e0d..ec2ff7acb21 100644
--- a/src/bin/pg_upgrade/relfilenumber.c
+++ b/src/bin/pg_upgrade/relfilenumber.c
@@ -18,7 +18,7 @@
#include "pg_upgrade.h"
static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace);
-static void transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit);
+static void transfer_relfile(FileNameMap *map, const char *type_suffix);
/*
* The following set of sync_queue_* functions are used for --swap to reduce
@@ -496,25 +496,10 @@ transfer_single_new_db(FileNameMap *maps, int size,
char *old_tablespace, char *new_tablespace)
{
int mapnum;
- bool vm_must_add_frozenbit = false;
-
- /*
- * Do we need to rewrite visibilitymap?
- */
- if (old_cluster.controldata.cat_ver < VISIBILITY_MAP_FROZEN_BIT_CAT_VER &&
- new_cluster.controldata.cat_ver >= VISIBILITY_MAP_FROZEN_BIT_CAT_VER)
- vm_must_add_frozenbit = true;
/* --swap has its own subroutine */
if (user_opts.transfer_mode == TRANSFER_MODE_SWAP)
{
- /*
- * We don't support --swap to upgrade from versions that require
- * rewriting the visibility map. We should've failed already if
- * someone tries to do that.
- */
- Assert(!vm_must_add_frozenbit);
-
do_swap(maps, size, old_tablespace, new_tablespace);
return;
}
@@ -525,13 +510,13 @@ transfer_single_new_db(FileNameMap *maps, int size,
strcmp(maps[mapnum].old_tablespace, old_tablespace) == 0)
{
/* transfer primary file */
- transfer_relfile(&maps[mapnum], "", vm_must_add_frozenbit);
+ transfer_relfile(&maps[mapnum], "");
/*
* Copy/link any fsm and vm files, if they exist
*/
- transfer_relfile(&maps[mapnum], "_fsm", vm_must_add_frozenbit);
- transfer_relfile(&maps[mapnum], "_vm", vm_must_add_frozenbit);
+ transfer_relfile(&maps[mapnum], "_fsm");
+ transfer_relfile(&maps[mapnum], "_vm");
}
}
}
@@ -540,12 +525,10 @@ transfer_single_new_db(FileNameMap *maps, int size,
/*
* transfer_relfile()
*
- * Copy or link file from old cluster to new one. If vm_must_add_frozenbit
- * is true, visibility map forks are converted and rewritten, even in link
- * mode.
+ * Copy or link file from old cluster to new one.
*/
static void
-transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit)
+transfer_relfile(FileNameMap *map, const char *type_suffix)
{
char old_file[MAXPGPATH];
char new_file[MAXPGPATH];
@@ -604,14 +587,6 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
/* Copying files might take some time, so give feedback. */
pg_log(PG_STATUS, "%s", old_file);
- if (vm_must_add_frozenbit && strcmp(type_suffix, "_vm") == 0)
- {
- /* Need to rewrite visibility map format */
- pg_log(PG_VERBOSE, "rewriting \"%s\" to \"%s\"",
- old_file, new_file);
- rewriteVisibilityMap(old_file, new_file, map->nspname, map->relname);
- }
- else
switch (user_opts.transfer_mode)
{
case TRANSFER_MODE_CLONE:
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 047670d4acb..9e83d4659be 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -12,22 +12,6 @@
#include "fe_utils/string_utils.h"
#include "pg_upgrade.h"
-/*
- * version_hook functions for check_for_data_types_usage in order to determine
- * whether a data type check should be executed for the cluster in question or
- * not.
- */
-bool
-jsonb_9_4_check_applicable(ClusterInfo *cluster)
-{
- /* JSONB changed its storage format during 9.4 beta */
- if (GET_MAJOR_VERSION(cluster->major_version) == 904 &&
- cluster->controldata.cat_ver < JSONB_FORMAT_CHANGE_CAT_VER)
- return true;
-
- return false;
-}
-
/*
* Older servers can't support newer protocol versions, so their connection
* strings will need to lock max_protocol_version to 3.0.
@@ -46,117 +30,6 @@ protocol_negotiation_supported(const ClusterInfo *cluster)
return (GET_MAJOR_VERSION(cluster->major_version) >= 1100);
}
-/*
- * old_9_6_invalidate_hash_indexes()
- * 9.6 -> 10
- * Hash index binary format has changed from 9.6->10.0
- */
-void
-old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode)
-{
- int dbnum;
- FILE *script = NULL;
- bool found = false;
- char *output_path = "reindex_hash.sql";
-
- prep_status("Checking for hash indexes");
-
- for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
- {
- PGresult *res;
- bool db_used = false;
- int ntups;
- int rowno;
- int i_nspname,
- i_relname;
- DbInfo *active_db = &cluster->dbarr.dbs[dbnum];
- PGconn *conn = connectToServer(cluster, active_db->db_name);
-
- /* find hash indexes */
- res = executeQueryOrDie(conn,
- "SELECT n.nspname, c.relname "
- "FROM pg_catalog.pg_class c, "
- " pg_catalog.pg_index i, "
- " pg_catalog.pg_am a, "
- " pg_catalog.pg_namespace n "
- "WHERE i.indexrelid = c.oid AND "
- " c.relam = a.oid AND "
- " c.relnamespace = n.oid AND "
- " a.amname = 'hash'"
- );
-
- ntups = PQntuples(res);
- i_nspname = PQfnumber(res, "nspname");
- i_relname = PQfnumber(res, "relname");
- for (rowno = 0; rowno < ntups; rowno++)
- {
- found = true;
- if (!check_mode)
- {
- if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
- pg_fatal("could not open file \"%s\": %m", output_path);
- if (!db_used)
- {
- PQExpBufferData connectbuf;
-
- initPQExpBuffer(&connectbuf);
- appendPsqlMetaConnect(&connectbuf, active_db->db_name);
- fputs(connectbuf.data, script);
- termPQExpBuffer(&connectbuf);
- db_used = true;
- }
- fprintf(script, "REINDEX INDEX %s.%s;\n",
- quote_identifier(PQgetvalue(res, rowno, i_nspname)),
- quote_identifier(PQgetvalue(res, rowno, i_relname)));
- }
- }
-
- PQclear(res);
-
- if (!check_mode && db_used)
- {
- /* mark hash indexes as invalid */
- PQclear(executeQueryOrDie(conn,
- "UPDATE pg_catalog.pg_index i "
- "SET indisvalid = false "
- "FROM pg_catalog.pg_class c, "
- " pg_catalog.pg_am a, "
- " pg_catalog.pg_namespace n "
- "WHERE i.indexrelid = c.oid AND "
- " c.relam = a.oid AND "
- " c.relnamespace = n.oid AND "
- " a.amname = 'hash'"));
- }
-
- PQfinish(conn);
- }
-
- if (script)
- fclose(script);
-
- if (found)
- {
- report_status(PG_WARNING, "warning");
- if (check_mode)
- pg_log(PG_WARNING, "\n"
- "Your installation contains hash indexes. These indexes have different\n"
- "internal formats between your old and new clusters, so they must be\n"
- "reindexed with the REINDEX command. After upgrading, you will be given\n"
- "REINDEX instructions.");
- else
- pg_log(PG_WARNING, "\n"
- "Your installation contains hash indexes. These indexes have different\n"
- "internal formats between your old and new clusters, so they must be\n"
- "reindexed with the REINDEX command. The file\n"
- " %s\n"
- "when executed by psql by the database superuser will recreate all invalid\n"
- "indexes; until then, none of these indexes will be used.",
- output_path);
- }
- else
- check_ok();
-}
-
/*
* Callback function for processing results of query for
* report_extension_updates()'s UpgradeTask. If the query returned any rows,
--
2.50.1 (Apple Git-155)
--2xrO7hiVfr8aWtYg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v4-0003-psql-bump-minimum-supported-version-to-v10.patch
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2026-04-17 18:20 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-06 21:44 Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode Melanie Plageman <[email protected]>
2023-04-07 00:52 ` David Rowley <[email protected]>
2023-04-14 07:20 ` Peter Eisentraut <[email protected]>
2023-04-15 00:59 ` David Rowley <[email protected]>
2023-04-16 00:09 ` David Rowley <[email protected]>
2023-04-26 08:47 ` Masahiko Sawada <[email protected]>
2023-04-26 11:26 ` David Rowley <[email protected]>
2026-04-17 18:20 [PATCH v4 2/4] pg_upgrade: bump minimum supported version to v10 Nathan Bossart <[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