agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH v2] Check partitions not in use
4+ messages / 4 participants
[nested] [flat]
* [PATCH v2] Check partitions not in use
@ 2019-07-22 15:09 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Alvaro Herrera @ 2019-07-22 15:09 UTC (permalink / raw)
---
src/backend/commands/tablecmds.c | 56 +++++++++++++++++++++++++-------
1 file changed, 45 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f39f993e01..fd17aa4b5a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -354,6 +354,7 @@ static void ATSimplePermissions(Relation rel, int allowed_targets);
static void ATWrongRelkindError(Relation rel, int allowed_targets);
static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode);
+static void ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode);
static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
LOCKMODE lockmode);
static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
@@ -3421,8 +3422,7 @@ CheckTableNotInUse(Relation rel, const char *stmt)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_IN_USE),
/* translator: first %s is a SQL command, eg ALTER TABLE */
- errmsg("cannot %s \"%s\" because "
- "it is being used by active queries in this session",
+ errmsg("cannot %s \"%s\" because it is being used by active queries in this session",
stmt, RelationGetRelationName(rel))));
if (rel->rd_rel->relkind != RELKIND_INDEX &&
@@ -3431,8 +3431,7 @@ CheckTableNotInUse(Relation rel, const char *stmt)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_IN_USE),
/* translator: first %s is a SQL command, eg ALTER TABLE */
- errmsg("cannot %s \"%s\" because "
- "it has pending trigger events",
+ errmsg("cannot %s \"%s\" because it has pending trigger events",
stmt, RelationGetRelationName(rel))));
}
@@ -3910,16 +3909,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_AddIdentity:
ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ /* This command never recurses */
pass = AT_PASS_ADD_CONSTR;
break;
- case AT_DropIdentity:
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
- pass = AT_PASS_DROP;
- break;
case AT_SetIdentity:
ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ /* This command never recurses */
pass = AT_PASS_COL_ATTRS;
break;
+ case AT_DropIdentity:
+ ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ /* This command never recurses */
+ pass = AT_PASS_DROP;
+ break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATPrepDropNotNull(rel, recurse, recursing);
@@ -3985,7 +3987,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
- /* Recursion occurs during execution phase */
+ ATCheckPartitionsNotInUse(rel, lockmode);
+ /* Other recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
cmd->subtype = AT_DropConstraintRecurse;
@@ -5224,8 +5227,9 @@ ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode)
{
/*
- * Propagate to children if desired. Only plain tables and foreign tables
- * have children, so no need to search for other relkinds.
+ * Propagate to children if desired. Only plain tables, foreign tables
+ * and partitioned tables have children, so no need to search for other
+ * relkinds.
*/
if (recurse &&
(rel->rd_rel->relkind == RELKIND_RELATION ||
@@ -5259,6 +5263,36 @@ ATSimpleRecursion(List **wqueue, Relation rel,
}
}
+/*
+ * Obtain list of partitions of the given table, locking them all at the given
+ * lockmode and ensuring that they all pass CheckTableNotInUse.
+ *
+ * This function is a no-op if the given relation is not a partitioned table;
+ * in particular, nothing is done if it's a legacy inheritance parent.
+ */
+static void
+ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
+{
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ List *inh;
+ ListCell *cell;
+
+ inh = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
+ /* first element is the parent rel; must ignore it */
+ for_each_cell(cell, inh, list_second_cell(inh))
+ {
+ Relation childrel;
+
+ /* find_all_inheritors already got lock */
+ childrel = table_open(lfirst_oid(cell), NoLock);
+ CheckTableNotInUse(childrel, "ALTER TABLE");
+ table_close(childrel, NoLock);
+ }
+ list_free(inh);
+ }
+}
+
/*
* ATTypedTableRecursion
*
--
2.17.1
--uAKRQypu60I7Lcqm--
^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v3 07/10] Use "template" initdb in tests
@ 2023-02-03 05:51 Andres Freund <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Andres Freund @ 2023-02-03 05:51 UTC (permalink / raw)
Discussion: https://postgr.es/m/[email protected]
---
meson.build | 30 ++++++++++
src/test/perl/PostgreSQL/Test/Cluster.pm | 46 ++++++++++++++-
src/test/regress/pg_regress.c | 74 ++++++++++++++++++------
.cirrus.tasks.yml | 3 +-
src/Makefile.global.in | 52 +++++++++--------
5 files changed, 161 insertions(+), 44 deletions(-)
diff --git a/meson.build b/meson.build
index f5ec442f9a9..8b2b521a013 100644
--- a/meson.build
+++ b/meson.build
@@ -3070,8 +3070,10 @@ testport = 40000
test_env = environment()
temp_install_bindir = test_install_location / get_option('bindir')
+test_initdb_template = meson.build_root() / 'tmp_install' / 'initdb-template'
test_env.set('PG_REGRESS', pg_regress.full_path())
test_env.set('REGRESS_SHLIB', regress_module.full_path())
+test_env.set('INITDB_TEMPLATE', test_initdb_template)
# Test suites that are not safe by default but can be run if selected
# by the user via the whitespace-separated list in variable PG_TEST_EXTRA.
@@ -3086,6 +3088,34 @@ if library_path_var != ''
endif
+# Create (and remove old) initdb template directory. Tests use that, where
+# possible, to make it cheaper to run tests.
+#
+# Use python to remove the old cached initdb, as we cannot rely on a working
+# 'rm' binary on windows.
+test('initdb_cache',
+ python,
+ args: [
+ '-c', '''
+import shutil
+import sys
+import subprocess
+
+shutil.rmtree(sys.argv[1], ignore_errors=True)
+sp = subprocess.run(sys.argv[2:] + [sys.argv[1]])
+sys.exit(sp.returncode)
+''',
+ test_initdb_template,
+ temp_install_bindir / 'initdb',
+ '-A', 'trust', '-N', '--no-instructions'
+ ],
+ priority: setup_tests_priority - 1,
+ timeout: 300,
+ is_parallel: false,
+ env: test_env,
+ suite: ['setup'])
+
+
###############################################################
# Test Generation
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 5e161dbee60..4d449c35de9 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -522,8 +522,50 @@ sub init
mkdir $self->backup_dir;
mkdir $self->archive_dir;
- PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A',
- 'trust', '-N', @{ $params{extra} });
+ # If available and if there aren't any parameters, use a previously
+ # initdb'd cluster as a template by copying it. For a lot of tests, that's
+ # substantially cheaper. Do so only if there aren't parameters, it doesn't
+ # seem worth figuring out whether they affect compatibility.
+ #
+ # There's very similar code in pg_regress.c, but we can't easily
+ # deduplicate it until we require perl at build time.
+ if (defined $params{extra} or !defined $ENV{INITDB_TEMPLATE})
+ {
+ note("initializing database system by running initdb");
+ PostgreSQL::Test::Utils::system_or_bail('initdb', '-D', $pgdata, '-A',
+ 'trust', '-N', @{ $params{extra} });
+ }
+ else
+ {
+ my @copycmd;
+ my $expected_exitcode;
+
+ note("initializing database system by copying initdb template");
+
+ if ($PostgreSQL::Test::Utils::windows_os)
+ {
+ @copycmd = qw(robocopy /E /NJS /NJH /NFL /NDL /NP);
+ $expected_exitcode = 1; # 1 denotes files were copied
+ }
+ else
+ {
+ @copycmd = qw(cp -a);
+ $expected_exitcode = 0;
+ }
+
+ @copycmd = (@copycmd, $ENV{INITDB_TEMPLATE}, $pgdata);
+
+ my $ret = PostgreSQL::Test::Utils::system_log(@copycmd);
+
+ # See http://perldoc.perl.org/perlvar.html#%24CHILD_ERROR
+ if ($ret & 127 or $ret >> 8 != $expected_exitcode)
+ {
+ BAIL_OUT(
+ sprintf("failed to execute command \"%s\": $ret",
+ join(" ", @copycmd)));
+ }
+ }
+
PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS},
'--config-auth', $pgdata, @{ $params{auth_extra} });
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b68632320a7..407e3915cec 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2295,6 +2295,7 @@ regression_main(int argc, char *argv[],
FILE *pg_conf;
const char *env_wait;
int wait_seconds;
+ const char *initdb_template_dir;
/*
* Prepare the temp instance
@@ -2316,25 +2317,64 @@ regression_main(int argc, char *argv[],
if (!directory_exists(buf))
make_directory(buf);
- /* initdb */
initStringInfo(&cmd);
- appendStringInfo(&cmd,
- "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync",
- bindir ? bindir : "",
- bindir ? "/" : "",
- temp_instance);
- if (debug)
- appendStringInfo(&cmd, " --debug");
- if (nolocale)
- appendStringInfo(&cmd, " --no-locale");
- appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
- fflush(NULL);
- if (system(cmd.data))
+
+ /*
+ * Create data directory.
+ *
+ * If available, use a previously initdb'd cluster as a template by
+ * copying it. For a lot of tests, that's substantially cheaper.
+ *
+ * There's very similar code in Cluster.pm, but we can't easily de
+ * duplicate it until we require perl at build time.
+ */
+ initdb_template_dir = getenv("INITDB_TEMPLATE");
+ if (initdb_template_dir == NULL || nolocale || debug)
{
- bail("initdb failed\n"
- "# Examine \"%s/log/initdb.log\" for the reason.\n"
- "# Command was: %s",
- outputdir, cmd.data);
+ note("initializing database system by running initdb");
+
+ appendStringInfo(&cmd,
+ "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync",
+ bindir ? bindir : "",
+ bindir ? "/" : "",
+ temp_instance);
+ if (debug)
+ appendStringInfo(&cmd, " --debug");
+ if (nolocale)
+ appendStringInfo(&cmd, " --no-locale");
+ appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
+ fflush(NULL);
+ if (system(cmd.data))
+ {
+ bail("initdb failed\n"
+ "# Examine \"%s/log/initdb.log\" for the reason.\n"
+ "# Command was: %s",
+ outputdir, cmd.data);
+ }
+ }
+ else
+ {
+#ifndef WIN32
+ const char *copycmd = "cp -a \"%s\" \"%s/data\"";
+ int expected_exitcode = 0;
+#else
+ const char *copycmd = "robocopy /E /NJS /NJH /NFL /NDL /NP \"%s\" \"%s/data\"";
+ int expected_exitcode = 1; /* 1 denotes files were copied */
+#endif
+
+ note("initializing database system by copying initdb template");
+
+ appendStringInfo(&cmd,
+ copycmd,
+ initdb_template_dir,
+ temp_instance);
+ if (system(cmd.data) != expected_exitcode)
+ {
+ bail("copying of initdb template failed\n"
+ "# Examine \"%s/log/initdb.log\" for the reason.\n"
+ "# Command was: %s",
+ outputdir, cmd.data);
+ }
}
pfree(cmd.data);
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 0cf7ba77996..e137769850d 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -109,8 +109,9 @@ task:
test_minimal_script: |
su postgres <<-EOF
ulimit -c unlimited
+ meson test $MTEST_ARGS --suite setup
meson test $MTEST_ARGS --num-processes ${TEST_JOBS} \
- tmp_install cube/regress pg_ctl/001_start_stop
+ cube/regress pg_ctl/001_start_stop
EOF
on_failure:
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index df9f721a41a..0b4ca0eb6ae 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -397,30 +397,6 @@ check: temp-install
.PHONY: temp-install
-temp-install: | submake-generated-headers
-ifndef NO_TEMP_INSTALL
-ifneq ($(abs_top_builddir),)
-ifeq ($(MAKELEVEL),0)
- rm -rf '$(abs_top_builddir)'/tmp_install
- $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
- $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
- $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
-endif
-endif
-endif
-
-# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL
-# entries appear more than once in the tree, and parallel installs of the same
-# file can fail with EEXIST.
-checkprep:
- $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done)
-
-PROVE = @PROVE@
-# There are common routines in src/test/perl, and some test suites have
-# extra perl modules in their own directory.
-PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
-# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS.
-PROVE_FLAGS =
# prepend to path if already set, else just set it
define add_to_path
@@ -437,8 +413,36 @@ ld_library_path_var = LD_LIBRARY_PATH
with_temp_install = \
PATH="$(abs_top_builddir)/tmp_install$(bindir):$(CURDIR):$$PATH" \
$(call add_to_path,$(strip $(ld_library_path_var)),$(abs_top_builddir)/tmp_install$(libdir)) \
+ INITDB_TEMPLATE='$(abs_top_builddir)'/tmp_install/initdb-template \
$(with_temp_install_extra)
+temp-install: | submake-generated-headers
+ifndef NO_TEMP_INSTALL
+ifneq ($(abs_top_builddir),)
+ifeq ($(MAKELEVEL),0)
+ rm -rf '$(abs_top_builddir)'/tmp_install
+ $(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
+ $(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+ $(MAKE) -j1 $(if $(CHECKPREP_TOP),-C $(CHECKPREP_TOP),) checkprep >>'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+
+ $(with_temp_install) initdb -A trust -N --no-instructions '$(abs_top_builddir)'/tmp_install/initdb-template >>'$(abs_top_builddir)'/tmp_install/log/initdb-template.log 2>&1
+endif
+endif
+endif
+
+# Tasks to run serially at the end of temp-install. Some EXTRA_INSTALL
+# entries appear more than once in the tree, and parallel installs of the same
+# file can fail with EEXIST.
+checkprep:
+ $(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C '$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install || exit; done)
+
+PROVE = @PROVE@
+# There are common routines in src/test/perl, and some test suites have
+# extra perl modules in their own directory.
+PG_PROVE_FLAGS = -I $(top_srcdir)/src/test/perl/ -I $(srcdir)
+# User-supplied prove flags such as --verbose can be provided in PROVE_FLAGS.
+PROVE_FLAGS =
+
ifeq ($(enable_tap_tests),yes)
ifndef PGXS
--
2.38.0
--uh2yukyzfvojbe2k
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0008-ci-switch-tasks-to-debugoptimized-build.patch"
^ permalink raw reply [nested|flat] 4+ messages in thread
* allow spread checkpoints when changing checksums online
@ 2026-05-04 13:42 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Tomas Vondra @ 2026-05-04 13:42 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Here's a small patch re-introducing the option to use spread checkpoints
(instead of always using CHECKPOINT_FAST) for online checksum changes.
The version v20251201 posted in [1] supported this, but the next patch
version was without checkpoints and so removed the "fast" parameter too.
Then we realized the checkpoints are actually needed, but were added
back to keep it as simple as possible. Or maybe it was an omission, not
sure, and there's no explanation on the thread.
I recall someone claiming always doing fast checkpoints is fine, because
we've already written the whole database into WAL anyway, and on large
databases that's likely way more expensive than a single checkpoint. I
don't buy that, for two reasons:
- We do have throttling for the rewrite phase, thanks to the cost_limit
and cost_delay parameters. So we can effectively throttle it, to reduce
impact of the checksums change. In which case the "fast" checkpoint can
be way more disruptive.
- We need to do checkpoints even when "disabling" checksums, in which
case we don't rewrite any data pages (or WAL-log anything), we just need
to persist the new checksum state. Which just makes the fast checkpoint
relatively more disruptive.
The attached patch is mostly extracted from v20251201, and adds the
"fast" parameter back to pg_{enable,disable}_data_checksums.
I have two open questions regarding it:
1) What should be the default? I've used fast=true, mostly because
that's what PG19 is going to do (fast checkpoints by default). It's also
somewhat consistent with e.g. VACUUM which does no throttling by
default. But I assume most production uses would want fast=false?
2) I haven't adjusted the TAP tests. We could use fast=false in a couple
of the test_checksums tests, but I'm not sure it's worth it and it makes
it way more time consuming.
We could reduce checkpoint_timeout to something very aggressive. And in
fact that's what I did locally with the TAP tests I posted in [2]. But
I'm still not convinced it's worth it - the checkpoints are still
synchronous, of course.
regards
[1]
https://www.postgresql.org/message-id/477897AE-1314-4724-9694-0BABC4F4ABDA%40yesql.se
[2]
https://www.postgresql.org/message-id/9e1331e1-93a0-4e27-934a-17b89342be4d%40vondra.me
--
Tomas Vondra
Attachments:
[text/x-patch] 0001-add-fast-parameter-to-enable-disable-checksums.patch (12.6K, ../../[email protected]/2-0001-add-fast-parameter-to-enable-disable-checksums.patch)
download | inline diff:
From 4eba7bbb739f5e7b95c4909d1cee2a437d8c6eb3 Mon Sep 17 00:00:00 2001
From: test <test>
Date: Sun, 3 May 2026 00:40:17 +0200
Subject: [PATCH 1/2] add fast parameter to enable/disable checksums
XXX Should this default to true or false? Most production systems would
probably want 'false' to reduce impact, but PG19 does fast checkpoints
by default.
---
doc/src/sgml/func/func-admin.sgml | 14 ++++++++++--
src/backend/access/transam/xlog.c | 23 ++++++++++++++------
src/backend/postmaster/datachecksum_state.c | 24 +++++++++++++++------
src/include/access/xlog.h | 4 ++--
src/include/catalog/pg_proc.dat | 11 +++++-----
src/include/postmaster/datachecksum_state.h | 3 ++-
6 files changed, 56 insertions(+), 23 deletions(-)
diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml
index 24ecb46542e..06360ea1b95 100644
--- a/doc/src/sgml/func/func-admin.sgml
+++ b/doc/src/sgml/func/func-admin.sgml
@@ -3158,7 +3158,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
<indexterm>
<primary>pg_enable_data_checksums</primary>
</indexterm>
- <function>pg_enable_data_checksums</function> ( <optional><parameter>cost_delay</parameter> <type>int</type>, <parameter>cost_limit</parameter> <type>int</type></optional> )
+ <function>pg_enable_data_checksums</function> ( <optional><parameter>cost_delay</parameter> <type>int</type>, <parameter>cost_limit</parameter> <type>int</type></optional>, <parameter>fast</parameter> <type>bool</type></optional> )
<returnvalue>void</returnvalue>
</para>
<para>
@@ -3175,6 +3175,11 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
specified, the process is throttled using the same principles as
<link linkend="runtime-config-resource-vacuum-cost">Cost-based Vacuum Delay</link>.
</para>
+ <para>
+ If <parameter>fast</parameter> is specified as <literal>true</literal>
+ then a fast checkpoint will be issued when data checksums have been
+ enabled, which may cause a spike in I/O.
+ </para>
</entry>
</row>
@@ -3183,7 +3188,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
<indexterm>
<primary>pg_disable_data_checksums</primary>
</indexterm>
- <function>pg_disable_data_checksums</function> ()
+ <function>pg_disable_data_checksums</function> ( <optional><parameter>fast</parameter> <type>bool</type></optional> )
<returnvalue>void</returnvalue>
</para>
<para>
@@ -3193,6 +3198,11 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
stopped validating data checksums, the data checksum state will be
set to <literal>off</literal>.
</para>
+ <para>
+ If <parameter>fast</parameter> is specified as <literal>true</literal>
+ then a fast checkpoint will be issued when data checksums have been
+ enabled, which may cause a spike in I/O.
+ </para>
</entry>
</row>
</tbody>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 18d5dee06e0..6b8b48fa4ce 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4803,9 +4803,10 @@ SetDataChecksumsOnInProgress(void)
* state transition.
*/
void
-SetDataChecksumsOn(void)
+SetDataChecksumsOn(bool fast)
{
uint64 barrier;
+ int flags;
SpinLockAcquire(&XLogCtl->info_lck);
@@ -4820,7 +4821,7 @@ SetDataChecksumsOn(void)
SpinLockRelease(&XLogCtl->info_lck);
elog(WARNING,
"cannot set data checksums to \"on\", current state is not \"inprogress-on\", disabling");
- SetDataChecksumsOff();
+ SetDataChecksumsOff(fast);
return;
}
@@ -4850,7 +4851,11 @@ SetDataChecksumsOn(void)
MyProc->delayChkptFlags &= ~DELAY_CHKPT_START;
END_CRIT_SECTION();
- RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT | CHECKPOINT_FAST);
+ flags = CHECKPOINT_FORCE | CHECKPOINT_WAIT;
+ if (fast)
+ flags |= CHECKPOINT_FAST;
+
+ RequestCheckpoint(flags);
WaitForProcSignalBarrier(barrier);
}
@@ -4868,9 +4873,15 @@ SetDataChecksumsOn(void)
* state transition.
*/
void
-SetDataChecksumsOff(void)
+SetDataChecksumsOff(bool fast)
{
uint64 barrier;
+ int flags;
+
+ /* determine flags for the checkpoint(s) */
+ flags = CHECKPOINT_FORCE | CHECKPOINT_WAIT;
+ if (fast)
+ flags |= CHECKPOINT_FAST;
SpinLockAcquire(&XLogCtl->info_lck);
@@ -4912,7 +4923,7 @@ SetDataChecksumsOff(void)
MyProc->delayChkptFlags &= ~DELAY_CHKPT_START;
END_CRIT_SECTION();
- RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT | CHECKPOINT_FAST);
+ RequestCheckpoint(flags);
WaitForProcSignalBarrier(barrier);
/*
@@ -4950,7 +4961,7 @@ SetDataChecksumsOff(void)
MyProc->delayChkptFlags &= ~DELAY_CHKPT_START;
END_CRIT_SECTION();
- RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT | CHECKPOINT_FAST);
+ RequestCheckpoint(flags);
WaitForProcSignalBarrier(barrier);
}
diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index d0d6acdd6a2..286096217d9 100644
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -291,6 +291,7 @@ typedef struct DataChecksumsStateStruct
DataChecksumsWorkerOperation launch_operation;
int launch_cost_delay;
int launch_cost_limit;
+ bool launch_fast_checkpoint;
/*
* Is a launcher process currently running? This is set by the main
@@ -318,6 +319,7 @@ typedef struct DataChecksumsStateStruct
DataChecksumsWorkerOperation operation;
int cost_delay;
int cost_limit;
+ bool fast_checkpoint;
/*
* Signaling between the launcher and the worker process.
@@ -509,6 +511,8 @@ AbsorbDataChecksumsBarrier(ProcSignalBarrierType barrier)
Datum
disable_data_checksums(PG_FUNCTION_ARGS)
{
+ bool fast = PG_GETARG_BOOL(0);
+
PreventCommandDuringRecovery("pg_disable_data_checksums()");
if (!superuser())
@@ -516,7 +520,7 @@ disable_data_checksums(PG_FUNCTION_ARGS)
errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to change data checksum state"));
- StartDataChecksumsWorkerLauncher(DISABLE_DATACHECKSUMS, 0, 0);
+ StartDataChecksumsWorkerLauncher(DISABLE_DATACHECKSUMS, 0, 0, fast);
PG_RETURN_VOID();
}
@@ -530,6 +534,7 @@ enable_data_checksums(PG_FUNCTION_ARGS)
{
int cost_delay = PG_GETARG_INT32(0);
int cost_limit = PG_GETARG_INT32(1);
+ bool fast = PG_GETARG_BOOL(2);
PreventCommandDuringRecovery("pg_enable_data_checksums()");
@@ -548,7 +553,7 @@ enable_data_checksums(PG_FUNCTION_ARGS)
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cost limit must be greater than zero"));
- StartDataChecksumsWorkerLauncher(ENABLE_DATACHECKSUMS, cost_delay, cost_limit);
+ StartDataChecksumsWorkerLauncher(ENABLE_DATACHECKSUMS, cost_delay, cost_limit, fast);
PG_RETURN_VOID();
}
@@ -568,7 +573,8 @@ enable_data_checksums(PG_FUNCTION_ARGS)
void
StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
int cost_delay,
- int cost_limit)
+ int cost_limit,
+ bool fast)
{
BackgroundWorker bgw;
BackgroundWorkerHandle *bgw_handle;
@@ -588,6 +594,7 @@ StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
DataChecksumState->launch_operation = op;
DataChecksumState->launch_cost_delay = cost_delay;
DataChecksumState->launch_cost_limit = cost_limit;
+ DataChecksumState->launch_fast_checkpoint = fast;
/* Is the launcher already running? If so, what is it doing? */
running = DataChecksumState->launcher_running;
@@ -938,7 +945,7 @@ launcher_exit(int code, Datum arg)
* the state to off since processing cannot be resumed.
*/
if (DataChecksumsInProgressOn())
- SetDataChecksumsOff();
+ SetDataChecksumsOff(DataChecksumState->fast_checkpoint);
LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
launcher_running = false;
@@ -1081,6 +1088,7 @@ DataChecksumsWorkerLauncherMain(Datum arg)
DataChecksumState->operation = operation;
DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
+ DataChecksumState->fast_checkpoint = DataChecksumState->launch_fast_checkpoint;
LWLockRelease(DataChecksumsWorkerLock);
/*
@@ -1139,7 +1147,7 @@ again:
* Data checksums have been set on all pages, set the state to on in
* order to instruct backends to validate checksums on reading.
*/
- SetDataChecksumsOn();
+ SetDataChecksumsOn(DataChecksumState->fast_checkpoint);
ereport(LOG,
errmsg("data checksums are now enabled"));
@@ -1151,7 +1159,7 @@ again:
pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_PHASE,
PROGRESS_DATACHECKSUMS_PHASE_DISABLING);
- SetDataChecksumsOff();
+ SetDataChecksumsOff(DataChecksumState->fast_checkpoint);
ereport(LOG,
errmsg("data checksums are now disabled"));
}
@@ -1179,6 +1187,7 @@ done:
operation = DataChecksumState->launch_operation;
DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
+ DataChecksumState->fast_checkpoint = DataChecksumState->launch_fast_checkpoint;
LWLockRelease(DataChecksumsWorkerLock);
goto again;
}
@@ -1268,7 +1277,7 @@ ProcessAllDatabases(void)
* Disable checksums on cluster, because we failed one of the
* databases and this is an all or nothing process.
*/
- SetDataChecksumsOff();
+ SetDataChecksumsOff(DataChecksumState->fast_checkpoint);
ereport(ERROR,
errcode(ERRCODE_INSUFFICIENT_RESOURCES),
errmsg("data checksums failed to get enabled in all databases, aborting"),
@@ -1606,6 +1615,7 @@ DataChecksumsWorkerMain(Datum arg)
DataChecksumState->cost_delay = DataChecksumState->launch_cost_delay;
DataChecksumState->cost_limit = DataChecksumState->launch_cost_limit;
+ DataChecksumState->fast_checkpoint = DataChecksumState->launch_fast_checkpoint;
}
else
costs_updated = false;
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 4dd98624204..8a9e8961a6a 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -253,8 +253,8 @@ extern bool DataChecksumsOn(void);
extern bool DataChecksumsOff(void);
extern bool DataChecksumsInProgressOn(void);
extern void SetDataChecksumsOnInProgress(void);
-extern void SetDataChecksumsOn(void);
-extern void SetDataChecksumsOff(void);
+extern void SetDataChecksumsOn(bool fast);
+extern void SetDataChecksumsOff(bool fast);
extern const char *show_data_checksums(void);
extern const char *get_checksum_state_string(uint32 state);
extern void InitLocalDataChecksumState(void);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fa9ae79082b..fed5fba4c0c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12457,14 +12457,15 @@
{ oid => '9258',
descr => 'disable data checksums',
proname => 'pg_disable_data_checksums', provolatile => 'v', prorettype => 'void',
- proparallel => 'r', prosrc => 'disable_data_checksums', proargtypes => '',
- proacl => '{POSTGRES=X}'},
+ proparallel => 'r', prosrc => 'disable_data_checksums', proargtypes => 'bool',
+ proallargtypes => '{bool}', proargmodes => '{i}', proargnames => '{fast}',
+ proargdefaults => '{true}', proacl => '{POSTGRES=X}'},
{ oid => '9257',
descr => 'enable data checksums',
proname => 'pg_enable_data_checksums', provolatile => 'v', prorettype => 'void',
- proparallel => 'r', proargtypes => 'int4 int4', proallargtypes => '{int4,int4}',
- proargmodes => '{i,i}', proargnames => '{cost_delay,cost_limit}',
- proargdefaults => '{0,100}', prosrc => 'enable_data_checksums',
+ proparallel => 'r', proargtypes => 'int4 int4 bool', proallargtypes => '{int4,int4,bool}',
+ proargmodes => '{i,i,i}', proargnames => '{cost_delay,cost_limit,fast}',
+ proargdefaults => '{0,100,true}', prosrc => 'enable_data_checksums',
proacl => '{POSTGRES=X}'},
# collation management functions
diff --git a/src/include/postmaster/datachecksum_state.h b/src/include/postmaster/datachecksum_state.h
index 2a1ae10d55d..6bd975c0670 100644
--- a/src/include/postmaster/datachecksum_state.h
+++ b/src/include/postmaster/datachecksum_state.h
@@ -45,7 +45,8 @@ void EmitAndWaitDataChecksumsBarrier(uint32 state);
/* Start the background processes for enabling or disabling checksums */
void StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op,
int cost_delay,
- int cost_limit);
+ int cost_limit,
+ bool fast);
/* Background worker entrypoints */
void DataChecksumsWorkerLauncherMain(Datum arg);
--
2.54.0
^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v1 2/2] run pgindent
@ 2026-05-05 21:04 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Nathan Bossart @ 2026-05-05 21:04 UTC (permalink / raw)
---
src/backend/replication/logical/logicalfuncs.c | 2 +-
src/backend/storage/ipc/dsm_registry.c | 2 +-
src/bin/pg_basebackup/pg_basebackup.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 512013b0ef0..71fbaf72269 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -218,7 +218,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
* what we need.
*/
if (!binary &&
- ctx->options.output_type !=OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
+ ctx->options.output_type != OUTPUT_PLUGIN_TEXTUAL_OUTPUT)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("logical decoding output plugin \"%s\" produces binary output, but function \"%s\" expects textual data",
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index 2b56977659b..b9961c26019 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -479,7 +479,7 @@ pg_get_dsm_registry_allocations(PG_FUNCTION_ARGS)
entry->dsa.handle != DSA_HANDLE_INVALID)
vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsa.handle));
else if (entry->type == DSMR_ENTRY_TYPE_DSH &&
- entry->dsh.dsa_handle !=DSA_HANDLE_INVALID)
+ entry->dsh.dsa_handle != DSA_HANDLE_INVALID)
vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsh.dsa_handle));
else
nulls[2] = true;
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index c1a4672aa6f..80dc3bbc8da 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1282,7 +1282,7 @@ ReceiveArchiveStream(PGconn *conn, pg_compress_specification *compress)
ReceiveCopyData(conn, ReceiveArchiveStreamChunk, &state);
/* If we wrote the backup manifest to a file, close the file. */
- if (state.manifest_file !=NULL)
+ if (state.manifest_file != NULL)
{
fclose(state.manifest_file);
state.manifest_file = NULL;
@@ -1341,7 +1341,7 @@ ReceiveArchiveStreamChunk(size_t r, char *copybuf, void *callback_data)
/* Sanity check. */
if (state->manifest_buffer != NULL ||
- state->manifest_file !=NULL)
+ state->manifest_file != NULL)
pg_fatal("archives must precede manifest");
/* Parse the rest of the CopyData message. */
@@ -1406,7 +1406,7 @@ ReceiveArchiveStreamChunk(size_t r, char *copybuf, void *callback_data)
appendPQExpBuffer(state->manifest_buffer, copybuf + 1,
r - 1);
}
- else if (state->manifest_file !=NULL)
+ else if (state->manifest_file != NULL)
{
/* Manifest data, write to disk. */
if (fwrite(copybuf + 1, r - 1, 1,
--
2.50.1 (Apple Git-155)
--k2Vi/uo5iFea1Gni--
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-05-05 21:04 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 15:09 [PATCH v2] Check partitions not in use Alvaro Herrera <[email protected]>
2023-02-03 05:51 [PATCH v3 07/10] Use "template" initdb in tests Andres Freund <[email protected]>
2026-05-04 13:42 allow spread checkpoints when changing checksums online Tomas Vondra <[email protected]>
2026-05-05 21:04 [PATCH v1 2/2] run pgindent 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