Re: minor bug
If we never expect getRecordTimestamp to fail, then why put it in the if-condition? getRecordTimestamp can fail if the record is not a restore point nor a commit or abort record. A few lines before in the same function there is this: /* Otherwise we only consider stopping before COMMIT or ABORT records. */ if (XLogRecGetRmid(record) != RM_XACT_ID) return false; I guess that make sure getRecordTimestamp can never fail. The way it is written in your patch invites it to be optimized out again. The only thing that prevents it is the comment. Why not (void)getRecordTimestamp(record, &recordXtime); if (recoveryTarget == RECOVERY_TARGET_TIME) ... On Wed, Jan 18, 2023 at 9:03 PM Tom Lane wrote: > Laurenz Albe writes: > > On Tue, 2023-01-17 at 10:32 -0500, Tom Lane wrote: > >> I seem to recall that the original idea was to report the timestamp > >> of the commit/abort record we are stopping at. Maybe my memory is > >> faulty, but I think that'd be significantly more useful than the > >> current behavior, *especially* when the replay stopping point is > >> defined by something other than time. > >> (Also, the wording of the log message suggests that that's what > >> the reported time is supposed to be. I wonder if somebody messed > >> this up somewhere along the way.) > > > recoveryStopTime is set to recordXtime (the time of the xlog record) > > a few lines above that patch, so this is useful information if it is > > present. > > Ah, but that only happens if recoveryTarget == RECOVERY_TARGET_TIME. > Digging in the git history, I see that this did use to work as > I remember: we always extracted the record time before printing it. > That was accidentally broken in refactoring in c945af80c. I think > the correct fix is more like the attached. > > regards, tom lane > >
Re: Allowing pg_recvlogical to create temporary replication slots
On Wed, Oct 30, 2024 at 9:01 AM Peter Eisentraut wrote: > On 27.10.24 13:37, Torsten Förtsch wrote: > > The attached patch enables pg_recvlogical to create a temporary slot. > > I think you should explain a bit why you want to do that, what use case > or scenario this is meant to address. > In my particular case I want to filter for messages sent by pg_logical_emit_message(). I don't care much about getting ALL the changes. If the replication slot disappears and a few changes are lost it does not matter. So, a temporary rep slot makes more sense than creating one and then having to make sure it is not retaining wal forever later. I can imagine this also as a tool to monitor changes for a while and then simply disconnect without the need to remove the slot. Why am I interested in pg_logical_emit_message()? We have an application with relatively complicated transactions involving multiple tables. Some of them use pg_notify(). We also have synchronous replication. Sometimes I see lock avalanches that can be traced to the "AccessExclusiveLock on object 0 of class 1262 of database 0". This lock is taken during commit when the notification is inserted in the queue. After that the backend waits for the confirmation by the sync replica. So, this lock presses all the transactions sending notifications into a sequence. Now, if the application uses pg_logical_emit_message() instead, then I think there is no equivalent lock. I understand the semantics are a bit different (timing) but close enough for my use case. Another advantage of pg_logical_emit_message() is the ability to send them even if the transaction is aborted. I was in the process of experimenting with this idea and found that pg_recvlogical can: - only create the slot or - create the slot and immediately use it - try to create the slot and if the slot is already there use it So, why not also allow it to create a temporary slot?
Re: Allowing pg_recvlogical to create temporary replication slots
This is another version of the patch. It now includes tests.
On Fri, Nov 1, 2024 at 1:42 PM Torsten Förtsch
wrote:
> On Wed, Oct 30, 2024 at 9:01 AM Peter Eisentraut
> wrote:
>
>> On 27.10.24 13:37, Torsten Förtsch wrote:
>> > The attached patch enables pg_recvlogical to create a temporary slot.
>>
>> I think you should explain a bit why you want to do that, what use case
>> or scenario this is meant to address.
>>
>
> In my particular case I want to filter for messages sent by
> pg_logical_emit_message(). I don't care much about getting ALL the changes.
> If the replication slot disappears and a few changes are lost it does not
> matter. So, a temporary rep slot makes more sense than creating one and
> then having to make sure it is not retaining wal forever later.
>
> I can imagine this also as a tool to monitor changes for a while and then
> simply disconnect without the need to remove the slot.
>
> Why am I interested in pg_logical_emit_message()? We have an application
> with relatively complicated transactions involving multiple tables. Some of
> them use pg_notify(). We also have synchronous replication. Sometimes I see
> lock avalanches that can be traced to the "AccessExclusiveLock on object 0
> of class 1262 of database 0". This lock is taken during commit when the
> notification is inserted in the queue. After that the backend waits for the
> confirmation by the sync replica. So, this lock presses all the
> transactions sending notifications into a sequence.
>
> Now, if the application uses pg_logical_emit_message() instead, then I
> think there is no equivalent lock. I understand the semantics are a bit
> different (timing) but close enough for my use case. Another advantage of
> pg_logical_emit_message() is the ability to send them even if the
> transaction is aborted.
>
> I was in the process of experimenting with this idea and found that
> pg_recvlogical can:
> - only create the slot or
> - create the slot and immediately use it
> - try to create the slot and if the slot is already there use it
>
> So, why not also allow it to create a temporary slot?
>
From 82d6256fb621e2dc75a67603bef5b511cfdf0e27 Mon Sep 17 00:00:00 2001
From: Torsten Foertsch
Date: Fri, 1 Nov 2024 13:56:43 +0100
Subject: [PATCH v2] Allow pg_recvlogical to create temp slots
With this patch pg_recvlogical can be called with the
--temporary-slot option together with --create-slot and --start.
If called that way, the created slot exists only for the duration
of the connection. If the connection is dropped and reestablished
by pg_recvlogical, a new temp slot by the same name is created.
If the slot exists and --if-not-exists is not passed, then
pg_recvlogical fails. If the slot exists and --if-not-exists is
given, the slot will be used.
In addition a few tests have been added for previously untested
options.
Discussion: https://www.postgresql.org/message-id/CAKkG4_%3DoMpa-AXhw9m044ZH5YdneNFTp6WxG_kEPA0cTkfiMNQ%40mail.gmail.com
---
src/bin/pg_basebackup/pg_recvlogical.c| 33 --
src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 65 +++
2 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 3db520ed38..4a131cca6d 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -50,6 +50,7 @@ static int fsync_interval = 10 * 1000; /* 10 sec = default */
static XLogRecPtr startpos = InvalidXLogRecPtr;
static XLogRecPtr endpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
+static bool slot_is_temporary = false;
static bool slot_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
@@ -104,6 +105,7 @@ usage(void)
printf(_(" -s, --status-interval=SECS\n"
" time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
printf(_(" -S, --slot=SLOTNAMEname of the logical replication slot\n"));
+ printf(_(" --temporary-slot the slot created exists until the connection is dropped\n"));
printf(_(" -t, --two-phaseenable decoding of prepared transactions when creating a slot\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -216,7 +218,7 @@ StreamLogicalLog(void)
char *copybuf = NULL;
TimestampTz last_status = -1;
int i;
- PQExpBuffer query;
+ PQExpBuffer query = NULL;
XLogRecPtr cur_record_lsn;
output_written_lsn = InvalidXLogRecPtr;
@@ -227,10 +229,24 @@ StreamLogicalLog(void)
* Connect in replication mode to the server
*/
if (!conn)
+ {
conn = GetConnection
Allowing pg_recvlogical to create temporary replication slots
Hi,
This is my very first message to this mailing list. Please advise if I am
making any mistakes in the procedure.
The attached patch enables pg_recvlogical to create a temporary slot.
What is the next step in the process to get this change into official
postgres?
Thanks,
Torsten
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 3db520ed38..22ab96129c 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -50,6 +50,7 @@ static int fsync_interval = 10 * 1000; /* 10 sec = default */
static XLogRecPtr startpos = InvalidXLogRecPtr;
static XLogRecPtr endpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
+static bool slot_is_temporary = false;
static bool slot_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
@@ -104,6 +105,7 @@ usage(void)
printf(_(" -s, --status-interval=SECS\n"
" time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
printf(_(" -S, --slot=SLOTNAMEname of the logical replication slot\n"));
+ printf(_(" --temporary-slot the slot created by --create-slot exists until the connection is dropped\n"));
printf(_(" -t, --two-phaseenable decoding of prepared transactions when creating a slot\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -227,10 +229,24 @@ StreamLogicalLog(void)
* Connect in replication mode to the server
*/
if (!conn)
+ {
conn = GetConnection();
- if (!conn)
- /* Error message already written in GetConnection() */
- return;
+ if (!conn)
+ /* Error message already written in GetConnection() */
+ return;
+
+ /* Recreate a replication slot. */
+ if (do_create_slot && slot_is_temporary)
+ {
+ if (verbose)
+pg_log_info("recreating replication slot \"%s\"", replication_slot);
+
+ if (!CreateReplicationSlot(conn, replication_slot, plugin, slot_is_temporary,
+ false, false, slot_exists_ok, two_phase))
+goto error;
+ startpos = InvalidXLogRecPtr;
+ }
+ }
/*
* Start the replication
@@ -719,6 +735,7 @@ main(int argc, char **argv)
{"start", no_argument, NULL, 2},
{"drop-slot", no_argument, NULL, 3},
{"if-not-exists", no_argument, NULL, 4},
+ {"temporary-slot", no_argument, NULL, 5},
{NULL, 0, NULL, 0}
};
int c;
@@ -847,6 +864,9 @@ main(int argc, char **argv)
case 4:
slot_exists_ok = true;
break;
+ case 5:
+slot_is_temporary = true;
+break;
default:
/* getopt_long already emitted a complaint */
@@ -981,7 +1001,7 @@ main(int argc, char **argv)
if (verbose)
pg_log_info("creating replication slot \"%s\"", replication_slot);
- if (!CreateReplicationSlot(conn, replication_slot, plugin, false,
+ if (!CreateReplicationSlot(conn, replication_slot, plugin, slot_is_temporary,
false, false, slot_exists_ok, two_phase))
exit(1);
startpos = InvalidXLogRecPtr;
Re: Allowing pg_recvlogical to create temporary replication slots
Hi Dickson,
>
> While reviewing your patch for pg_recvlogical, I’m curious about the
> expected behavior when using --if-not-exists in conjunction with
> --temporary-slot.
>
> If an existing slot is reused under these options, will it behave as
> a temporary slot and be removed when the connection ends? Or the
> existing slot should remain persistent despite the --temporary-slot flag?
>
If the slot did not exist before, it is created as a temporary slot. If
subsequently the connection drops and pg_recvlogical reconnects (no -n
option), the slot is created again as a temporary slot. That means changes
happening in between will not be captured.
If the slot existed before, the existing slot will be used, no error is
generated. In that case the slot will keep existing when the connection is
dropped.
The following situation can happen:
- the slot does not exist and is created as a temporary slot
- the connection drops and pg_recvlogical tries to reconnect after a few
seconds
- in that period a permanent slot by the same name is created but no reader
is connected to it
- pg_recvlogical reconnects and finds the existing slot
In this case, the now existing slot is simply used. It will remain when the
connection is dropped.
Thanks for reviewing.
Basically, --if-not-exists flag tells pg_recvlogical to ignore the error it
would get when creating a slot with a name that's already there. The new
flag does not change that.
This is the relevant code in src/bin/pg_basebackup/streamutil.c checking
the result of the CREATE_REPLICATION_SLOT command.
if (slot_exists_ok &&
sqlstate &&
strcmp(sqlstate, ERRCODE_DUPLICATE_OBJECT) == 0)
{
...
return true;
}
PGSERVICEFILE as part of a normal connection string
Hi,
I like to bundle all my database connections in a .pg_service.conf. Over
time I collected a bunch of such service files. A while back I discovered
that the service file can only be specified as an environment variable. It
cannot be given as part of the connection string like
psql "service=$MY_SERVICE servicefile=MY_SERVICE_FILE"
The attached patch allows that.
Regards.
--
Torsten
From f419584d2fc7766c143d304ba2f2fad98501d9ea Mon Sep 17 00:00:00 2001
From: Torsten Foertsch
Date: Sat, 16 Nov 2024 20:17:20 +0100
Subject: [PATCH v1] PGSERVICEFILE as part of the connection string
Libpq interprets the PGSERVICEFILE environment variable. However,
the servicefile cannot be specified as part of the connection
string itself.
This change implements that.
---
doc/src/sgml/libpq.sgml | 16 -
src/interfaces/libpq/fe-connect.c | 18 -
src/interfaces/libpq/t/006_service.pl | 97 +++
3 files changed, 129 insertions(+), 2 deletions(-)
create mode 100644 src/interfaces/libpq/t/006_service.pl
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index bfefb1289e..de8830bf51 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2202,6 +2202,19 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
+
+ servicefile
+
+
+This option specifies the name of the per-user connection service file
+(see ).
+Defaults to ~/.pg_service.conf, or
+%APPDATA%\postgresql\.pg_service.conf on
+Microsoft Windows.
+
+
+
+
target_session_attrs
@@ -9337,7 +9350,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
On Microsoft Windows, it is named
%APPDATA%\postgresql\.pg_service.conf (where
%APPDATA% refers to the Application Data subdirectory
- in the user's profile). A different file name can be specified by
+ in the user's profile). A different file name can be specified using the
+ servicefile key word in a libpq connection string or by
setting the environment variable PGSERVICEFILE.
The system-wide file is named pg_service.conf.
By default it is sought in the etc directory
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 51083dcfd8..117588c0e8 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -192,6 +192,9 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
{"service", "PGSERVICE", NULL, NULL,
"Database-Service", "", 20, -1},
+ {"servicefile", "PGSERVICEFILE", NULL, NULL,
+ "Database-Service-File", "", 64, -1},
+
{"user", "PGUSER", NULL, NULL,
"Database-User", "", 20,
offsetof(struct pg_conn, pguser)},
@@ -5493,6 +5496,7 @@ static int
parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
{
const char *service = conninfo_getval(options, "service");
+ const char *service_fname = conninfo_getval(options, "servicefile");
char serviceFile[MAXPGPATH];
char *env;
bool group_found = false;
@@ -5515,7 +5519,9 @@ parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
* Try PGSERVICEFILE if specified, else try ~/.pg_service.conf (if that
* exists).
*/
- if ((env = getenv("PGSERVICEFILE")) != NULL)
+ if (service_fname != NULL)
+ strlcpy(serviceFile, service_fname, sizeof(serviceFile));
+ else if ((env = getenv("PGSERVICEFILE")) != NULL)
strlcpy(serviceFile, env, sizeof(serviceFile));
else
{
@@ -5678,6 +5684,16 @@ parseServiceFile(const char *serviceFile,
goto exit;
}
+if (strcmp(key, "servicefile") == 0)
+{
+ libpq_append_error(errorMessage,
+ "nested servicefile specifications not supported in service file \"%s\", line %d",
+ serviceFile,
+ linenr);
+ result = 3;
+ goto exit;
+}
+
/*
* Set the parameter --- but don't override any previous
* explicit setting.
diff --git a/src/interfaces/libpq/t/006_service.pl b/src/interfaces/libpq/t/006_service.pl
new file mode 100644
index 00..5de84b78af
--- /dev/null
+++ b/src/interfaces/libpq/t/006_service.pl
@@ -0,0 +1,97 @@
+# Copyright (c) 2023-2024, PostgreSQL Global Development Group
+use strict;
+use warnings FATAL => 'all';
+use Config;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+# This tests "service" and "servicefile"
+
+# Cluster setup which is shared for testing both load balancing methods
+my $node = PostgreSQL::Test::Cluster->new('node');
+
+# Create a data directory with initdb
+$node->init();
+
+# Start the PostgreSQL server
+$node->start();
+
+my $td = PostgreSQL::Test::Utils::tempdir;
+my $srvfile = "$td/pgsrv.conf";
+
+open my $fh, '>', $srvfile or die $!;
+print $fh "[my_srv]\n";
+print $fh +($node->connstr =~ s/ /\n/gr), "\n";
+close $fh;
+
+{
+ local $ENV{PGSERVICEFILE} = $srvfile;
+ $node->connect_ok(
+ 'service=my_srv',
+ 'service=my_srv',
+ sql => "SE
Re: Allowing pg_recvlogical to create temporary replication slots
On Tue, Nov 12, 2024 at 3:30 PM Dickson S. Guedes
wrote:
Are you planning to add changes in docs? Would be important to have
> this clarification in there also, IMHO.
>
Thanks. Here is an updated version of the patch including documentation.
From 80e2894d3289cebd9c12d457620bf2965cc0252d Mon Sep 17 00:00:00 2001
From: Torsten Foertsch
Date: Sat, 16 Nov 2024 12:42:11 +0100
Subject: [PATCH v3] Allow pg_recvlogical to create temp slots
With this patch pg_recvlogical can be called with the
--temporary-slot option together with --create-slot and --start.
If called that way, the created slot exists only for the duration
of the connection. If the connection is dropped and reestablished
by pg_recvlogical, a new temp slot by the same name is created.
If the slot exists and --if-not-exists is not passed, then
pg_recvlogical fails. If the slot exists and --if-not-exists is
given, the slot will be used.
In addition a few tests have been added for previously untested
options.
Discussion: https://www.postgresql.org/message-id/CAKkG4_%3DoMpa-AXhw9m044ZH5YdneNFTp6WxG_kEPA0cTkfiMNQ%40mail.gmail.com
---
doc/src/sgml/ref/pg_recvlogical.sgml | 25 +++
src/bin/pg_basebackup/pg_recvlogical.c| 33 --
src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 65 +++
3 files changed, 117 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml
index 95eb14b635..72e9952966 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -268,6 +268,31 @@ PostgreSQL documentation
+
+ --temporary-slot
+
+
+Create a temporary replication slot. This option is only useful if
+specified together with --create-slot and
+--start. A temporary slot is automatically dropped
+when the database connection is dropped.
+
+
+If used together with --if-not-exists and if a
+permanent slot by the requested name exists, that slot is used instead
+of creating a new one. That permanent slot is then not automatically
+removed when the connection is dropped.
+
+
+Unless --no-loop is specified,
+pg_recvlogical will try to reconnect automatically
+when the server connection is lost. If a temporary slot is requested,
+the slot will be recreated first. If at that time another slot by the
+same name exists, creation will fail.
+
+
+
+
-t
--two-phase
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 42b2a7bb9d..6ec02e427f 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -48,6 +48,7 @@ static int fsync_interval = 10 * 1000; /* 10 sec = default */
static XLogRecPtr startpos = InvalidXLogRecPtr;
static XLogRecPtr endpos = InvalidXLogRecPtr;
static bool do_create_slot = false;
+static bool slot_is_temporary = false;
static bool slot_exists_ok = false;
static bool do_start_slot = false;
static bool do_drop_slot = false;
@@ -102,6 +103,7 @@ usage(void)
printf(_(" -s, --status-interval=SECS\n"
" time between status packets sent to server (default: %d)\n"), (standby_message_timeout / 1000));
printf(_(" -S, --slot=SLOTNAMEname of the logical replication slot\n"));
+ printf(_(" --temporary-slot the slot created exists until the connection is dropped\n"));
printf(_(" -t, --two-phaseenable decoding of prepared transactions when creating a slot\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -214,7 +216,7 @@ StreamLogicalLog(void)
char *copybuf = NULL;
TimestampTz last_status = -1;
int i;
- PQExpBuffer query;
+ PQExpBuffer query = NULL;
XLogRecPtr cur_record_lsn;
output_written_lsn = InvalidXLogRecPtr;
@@ -225,10 +227,24 @@ StreamLogicalLog(void)
* Connect in replication mode to the server
*/
if (!conn)
+ {
conn = GetConnection();
- if (!conn)
- /* Error message already written in GetConnection() */
- return;
+ if (!conn)
+ /* Error message already written in GetConnection() */
+ return;
+
+ /* Recreate a replication slot. */
+ if (do_create_slot && slot_is_temporary)
+ {
+ if (verbose)
+pg_log_info("recreating replication slot \"%s\"", replication_slot);
+
+ if (!CreateReplicationSlot(conn, replication_slot, plugin, slot_is_temporary,
+ false, false, slot_exists_ok, two_phase))
+goto error;
+ startpos = InvalidXLogRecPtr;
+ }
+ }
/*
* Start the replication
@@ -654,7 +670,8 @@ error:
PQfreemem(copybuf);
copybuf = NULL;
}
- destroyPQExpBuffer(query);
+ if (query != NULL)
+ destroyPQExpBuffer(query);
PQfinish(conn);
conn = NULL;
}
@@ -717,6 +734,7 @@ main(int argc, char **argv)
