public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: 'Michael Paquier' <[email protected]>
Cc: 'Paul A Jungwirth' <[email protected]>
Cc: Mutaamba Maasha <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Bertrand Drouvot <[email protected]>
Cc: [email protected] <[email protected]>
Subject: RE: ReplicationSlotRelease() crashes when the instance is in the single user mode
Date: Tue, 19 Aug 2025 10:26:22 +0000
Message-ID: <OSCPR01MB14966D87D8EB0D9CD32C6A43DF530A@OSCPR01MB14966.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <[email protected]>
References: <OSCPR01MB14966F234CD3121F51810D5CCF5C52@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966FCE16ECCF29AE0502AF2F5C42@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<OSCPR01MB149660520A0E53CC53E8B66F1F5C42@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<CAA4eK1LK5nZUWwNY31X6BUXdQwwzpKGKxnw+zFf-cWKm3nuCEg@mail.gmail.com>
<OSCPR01MB149669CFD736D48C4450337DDF5CD2@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<CAA4eK1L8_QrYavkCy9b0Rrz8gqZRQE8kEmv3g7Tz6tsN8YuvhA@mail.gmail.com>
<OSCPR01MB149660CD4D6648CFE7FE68D34F5CC2@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<CA+renyWi+bbD8M8CDAaOMGbUrXptKLCK8zQWEhXy00PJuJqNuQ@mail.gmail.com>
<OSCPR01MB14966B39B3ED21229EAEFA10FF531A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
Dear Michael, Paul, Mutaamba,
> FWIW, I had my doubts at the beginning on the thread about the
> use-case, but after re-reading the whole thing I am going to side with
> Robert's opinion that if a fix to authorize some of the operations is
> simple, then let's just authorize the case(s) and have the fix.
OK, thanks for the clarification. Let's focus on 0001 now.
> Echoing with Paul, what I find as critically lacking from the proposed
> patches are regression tests to check and validate the behaviors you
> are looking for in the patches. Please add some in the shape of perl
> commands that use direct --single commands of postgres. We have cases
> that do so currently in the tree: sepgsql and shm. I don't see a
> reason why we could not do that as well here with run_log(). It is
> also possible to pass a -c exit_on_error=true to force failures.
>
> Not having a test for the single-user case of a slot drop is of course
> something that we are lacking now. As we are playing with this area
> of the code, let's add something for this case as well.
I added a first version of the test. It could work on my env (Linux).
Since I cannot come up the appropriate place, I introduced new test file in
recovery test. creating/dropping/advancing a slot, and doing a logical decoding
is tested. Since standby mode is not supported by the single-user instance,
I did not test the slot synchronization. I have not known that.
0002 was updated accordingly just in case. In the patch some functions were expected
to fail. Also, a check for pg_sync_replication_slots() was removed because we cannot call
the function in the first place.
IIUC, 0001 can be backpatched for all supported branches. I will create for them
after patch would be a good shape.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
[application/octet-stream] v6-0001-Set-ReplicationSlot-active_pid-even-in-single-use.patch (3.6K, ../OSCPR01MB14966D87D8EB0D9CD32C6A43DF530A@OSCPR01MB14966.jpnprd01.prod.outlook.com/2-v6-0001-Set-ReplicationSlot-active_pid-even-in-single-use.patch)
download | inline diff:
From e7d7de8fc99258ea826fc6269497313fefb417c1 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Mon, 18 Aug 2025 11:47:43 +0900
Subject: [PATCH v6 1/2] Set ReplicationSlot::active_pid even in single-user
mode
Slot manipulation functions except drop could be failed in single-user mode, because
active_pid was not set. Actually the attribute is no-op in the mode but fix the
inconsisntency anyway.
---
src/backend/replication/slot.c | 2 +-
src/test/recovery/meson.build | 3 +-
.../t/049_slots_in_single_user_mode.pl | 67 +++++++++++++++++++
3 files changed, 70 insertions(+), 2 deletions(-)
create mode 100644 src/test/recovery/t/049_slots_in_single_user_mode.pl
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 8605776ad86..fd0fdb96d42 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -653,7 +653,7 @@ retry:
}
else
{
- active_pid = MyProcPid;
+ s->active_pid = active_pid = MyProcPid;
ReplicationSlotSetInactiveSince(s, 0, true);
}
LWLockRelease(ReplicationSlotControlLock);
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index 52993c32dbb..7b1fe30f112 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -56,7 +56,8 @@ tests += {
't/045_archive_restartpoint.pl',
't/046_checkpoint_logical_slot.pl',
't/047_checkpoint_physical_slot.pl',
- 't/048_vacuum_horizon_floor.pl'
+ 't/048_vacuum_horizon_floor.pl',
+ 't/049_slots_in_single_user_mode.pl',
],
},
}
diff --git a/src/test/recovery/t/049_slots_in_single_user_mode.pl b/src/test/recovery/t/049_slots_in_single_user_mode.pl
new file mode 100644
index 00000000000..de32160be07
--- /dev/null
+++ b/src/test/recovery/t/049_slots_in_single_user_mode.pl
@@ -0,0 +1,67 @@
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Tests the slot manipulation in the single-user mode
+
+# Run passed commands in single-user mode. The return value from the command is
+# passed through.
+sub run_test_in_single_user_mode
+{
+ my ($node, $commands, $testname) = @_;
+
+ my $result = run_log(
+ [
+ 'postgres', '--single', '-F',
+ '-c' => 'exit_on_error=true',
+ '-D' => $node->data_dir,
+ 'postgres'
+ ],
+ \$commands);
+
+ ok($result, $testname);
+}
+
+my $slotname = 'test_slot';
+
+# Initialize a node
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init(allows_streaming => "logical");
+$node->start;
+
+# Define initial table
+$node->safe_psql('postgres', "CREATE TABLE foo (id int)");
+
+# Stop the node to run and test in single-user mode
+$node->stop;
+
+run_test_in_single_user_mode(
+ $node,
+ "SELECT pg_create_logical_replication_slot('$slotname', 'test_decoding')",
+ "replication slot can be created in single-user mode");
+
+run_test_in_single_user_mode(
+ $node, qq(
+INSERT INTO foo VALUES (1);
+SELECT count(1) FROM pg_logical_slot_get_changes('$slotname', NULL, NULL);
+),
+ "logical decoding be done in single-user mode");
+
+run_test_in_single_user_mode(
+ $node,
+ "SELECT pg_replication_slot_advance('$slotname', pg_current_wal_lsn())",
+ "replication slot can be advanced in single-user mode");
+
+run_test_in_single_user_mode(
+ $node,
+ "SELECT pg_copy_logical_replication_slot('$slotname', 'dest_slot')",
+ "replication slot can be copied in single-user mode");
+
+run_test_in_single_user_mode(
+ $node,
+ "SELECT pg_drop_replication_slot('$slotname')",
+ "replication slot can be dropped in single-user mode");
+
+done_testing();
--
2.47.1
[application/octet-stream] v6-0002-Prohibit-slot-manipulation-while-in-single-user-m.patch (8.4K, ../OSCPR01MB14966D87D8EB0D9CD32C6A43DF530A@OSCPR01MB14966.jpnprd01.prod.outlook.com/3-v6-0002-Prohibit-slot-manipulation-while-in-single-user-m.patch)
download | inline diff:
From 4ad717d5549cb6ba26a002b69aba7a148a1cd0fc Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Wed, 19 Feb 2025 11:37:26 +0900
Subject: [PATCH v6 2/2] Prohibit slot manipulation while in single-user mode
Replication-related commands are rarely used in single-user mode and have been
broken for years. This commit prohibits calling slot manipulation SQL functions
to avoid additional risk of failures. One exception is pg_drop_replication_slot.
It is still allowed because users may want to clean up their mistakes in the mode.
---
doc/src/sgml/func/func-admin.sgml | 5 ++
.../replication/logical/logicalfuncs.c | 3 ++
src/backend/replication/slot.c | 12 +++++
src/backend/replication/slotfuncs.c | 14 ++++++
src/backend/utils/adt/pg_upgrade_support.c | 3 ++
src/include/replication/slot.h | 1 +
.../t/049_slots_in_single_user_mode.pl | 48 ++++++++++++++-----
7 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml
index 446fdfe56f4..cfa09e4b987 100644
--- a/doc/src/sgml/func/func-admin.sgml
+++ b/doc/src/sgml/func/func-admin.sgml
@@ -1012,6 +1012,11 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
are also relevant for replication.
</para>
+ <para>
+ Note that slot manipulation functions except <function>pg_drop_replication_slot</function>
+ cannot be used in single-user mode.
+ </para>
+
<table id="functions-replication-table">
<title>Replication Management Functions</title>
<tgroup cols="1">
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index ca53caac2f2..30877fdc3ab 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -113,6 +113,9 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
List *options = NIL;
DecodingOutputState *p;
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
CheckSlotPermissions();
CheckLogicalDecodingRequirements();
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index fd0fdb96d42..4ca1227cc94 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1529,6 +1529,18 @@ CheckSlotPermissions(void)
"REPLICATION")));
}
+/*
+ * Check whether the instance is in single-user mode.
+ */
+void
+CheckSlotIsInSingleUserMode(void)
+{
+ if (!IsUnderPostmaster)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("replication slots cannot be used in single-user mode")));
+}
+
/*
* Reserve WAL for the currently active slot.
*
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 69f4c6157c5..90fe63283a1 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -17,6 +17,7 @@
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
#include "funcapi.h"
+#include "miscadmin.h"
#include "replication/logical.h"
#include "replication/slot.h"
#include "replication/slotsync.h"
@@ -76,6 +77,9 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
CheckSlotPermissions();
CheckSlotRequirements();
@@ -182,6 +186,9 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
CheckSlotPermissions();
CheckLogicalDecodingRequirements();
@@ -521,6 +528,9 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
Assert(!MyReplicationSlot);
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
CheckSlotPermissions();
if (XLogRecPtrIsInvalid(moveto))
@@ -618,9 +628,13 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
TupleDesc tupdesc;
HeapTuple tuple;
+
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
elog(ERROR, "return type must be a row type");
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
CheckSlotPermissions();
if (logical_slot)
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index a4f8b4faa90..337f705b34b 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -296,6 +296,9 @@ binary_upgrade_logical_slot_has_caught_up(PG_FUNCTION_ARGS)
*/
Assert(has_rolreplication(GetUserId()));
+ /* Slot manipulation is not allowed in single-user mode */
+ CheckSlotIsInSingleUserMode();
+
slot_name = PG_GETARG_NAME(0);
/* Acquire the given slot */
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index e8fc342d1a9..b4d89760f96 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -342,6 +342,7 @@ extern void CheckPointReplicationSlots(bool is_shutdown);
extern void CheckSlotRequirements(void);
extern void CheckSlotPermissions(void);
+extern void CheckSlotIsInSingleUserMode(void);
extern ReplicationSlotInvalidationCause
GetSlotInvalidationCause(const char *cause_name);
extern const char *GetSlotInvalidationCauseName(ReplicationSlotInvalidationCause cause);
diff --git a/src/test/recovery/t/049_slots_in_single_user_mode.pl b/src/test/recovery/t/049_slots_in_single_user_mode.pl
index de32160be07..e38a32472b2 100644
--- a/src/test/recovery/t/049_slots_in_single_user_mode.pl
+++ b/src/test/recovery/t/049_slots_in_single_user_mode.pl
@@ -21,9 +21,31 @@ sub run_test_in_single_user_mode
],
\$commands);
+ return $result;
+}
+
+# Wrapper function for run_test_in_single_user_mode. This would be used when
+# the input command succeeds.
+sub run_test_in_single_user_mode_success
+{
+ my ($node, $commands, $testname) = @_;
+
+ my $result = run_test_in_single_user_mode($node, $commands, $testname);
+
ok($result, $testname);
}
+# Wrapper function for run_test_in_single_user_mode. This would be used when
+# the input command fails.
+sub run_test_in_single_user_mode_fail
+{
+ my ($node, $commands, $testname) = @_;
+
+ my $result = run_test_in_single_user_mode($node, $commands, $testname);
+
+ ok(!$result, $testname);
+}
+
my $slotname = 'test_slot';
# Initialize a node
@@ -32,34 +54,38 @@ $node->init(allows_streaming => "logical");
$node->start;
# Define initial table
-$node->safe_psql('postgres', "CREATE TABLE foo (id int)");
+$node->safe_psql(
+ 'postgres', qq(
+CREATE TABLE foo (id int);
+SELECT pg_create_logical_replication_slot('$slotname', 'test_decoding');
+));
# Stop the node to run and test in single-user mode
$node->stop;
-run_test_in_single_user_mode(
+run_test_in_single_user_mode_fail(
$node,
- "SELECT pg_create_logical_replication_slot('$slotname', 'test_decoding')",
- "replication slot can be created in single-user mode");
+ "SELECT pg_create_logical_replication_slot('another_slot', 'test_decoding')",
+ "replication slot cannot be created in single-user mode");
-run_test_in_single_user_mode(
+run_test_in_single_user_mode_fail(
$node, qq(
INSERT INTO foo VALUES (1);
SELECT count(1) FROM pg_logical_slot_get_changes('$slotname', NULL, NULL);
),
- "logical decoding be done in single-user mode");
+ "logical decoding cannot be done in single-user mode");
-run_test_in_single_user_mode(
+run_test_in_single_user_mode_fail(
$node,
"SELECT pg_replication_slot_advance('$slotname', pg_current_wal_lsn())",
- "replication slot can be advanced in single-user mode");
+ "replication slot cannot be advanced in single-user mode");
-run_test_in_single_user_mode(
+run_test_in_single_user_mode_fail(
$node,
"SELECT pg_copy_logical_replication_slot('$slotname', 'dest_slot')",
- "replication slot can be copied in single-user mode");
+ "replication slot cannot be copied in single-user mode");
-run_test_in_single_user_mode(
+run_test_in_single_user_mode_success(
$node,
"SELECT pg_drop_replication_slot('$slotname')",
"replication slot can be dropped in single-user mode");
--
2.47.1
view thread (31+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: ReplicationSlotRelease() crashes when the instance is in the single user mode
In-Reply-To: <OSCPR01MB14966D87D8EB0D9CD32C6A43DF530A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox