public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bharath Rupireddy <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)
Date: Thu, 23 Mar 2023 23:03:38 +0530
Message-ID: <CALj2ACXvqFcxro64q4gjDeBNLgyPH0E6bG-WJ6w7qBqbggj3=w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA4eK1+CoHVhP6hs+5-_1_A9EymSyqDPzHxBs1jSrTm5fj6CvQ@mail.gmail.com>
<[email protected]>
On Thu, Mar 23, 2023 at 3:37 PM Alvaro Herrera <[email protected]> wrote:
>
> On 2023-Mar-22, Amit Kapila wrote:
>
> > I see that you have modified the patch to address the comments from
> > Alvaro. Personally, I feel it would be better to add such a message at
> > a centralized location instead of spreading these in different callers
> > of slot acquire/release functionality to avoid getting these missed in
> > the new callers in the future. However, if Alvaro and others think
> > that the current style is better then we should go ahead and do it
> > that way. I hope that we should be able to decide on this and get it
> > into PG16. Anyone else would like to weigh in here?
>
> I like Peter Smith's suggestion downthread.
+1. Please review the attached v8 patch further.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v8-0001-Add-LOG-messages-when-replication-slots-become-ac.patch (6.4K, ../CALj2ACXvqFcxro64q4gjDeBNLgyPH0E6bG-WJ6w7qBqbggj3=w@mail.gmail.com/2-v8-0001-Add-LOG-messages-when-replication-slots-become-ac.patch)
download | inline diff:
From b0824cbe21d02d6826aead6cc7cf50c955160626 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Thu, 23 Mar 2023 17:21:12 +0000
Subject: [PATCH v8] Add LOG messages when replication slots become active and
inactive
These logs will be extremely useful on production servers to debug
and analyze inactive replication slot issues.
---
doc/src/sgml/config.sgml | 7 +++--
src/backend/replication/slot.c | 45 +++++++++++++++++++++++++++++
src/backend/replication/walsender.c | 17 +++++++++++
src/backend/utils/misc/guc_tables.c | 2 +-
src/include/replication/slot.h | 2 ++
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 481f93cea1..f9b0d3c9a9 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7593,9 +7593,10 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
</term>
<listitem>
<para>
- Causes each replication command to be logged in the server log.
- See <xref linkend="protocol-replication"/> for more information about
- replication command. The default value is <literal>off</literal>.
+ Causes each replication command and related activity to be logged in
+ the server log. See <xref linkend="protocol-replication"/> for more
+ information about replication command. The default value is
+ <literal>off</literal>.
Only superusers and users with the appropriate <literal>SET</literal>
privilege can change this setting.
</para>
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2293c0c6fc..4dba028ab9 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -180,7 +180,16 @@ ReplicationSlotShmemExit(int code, Datum arg)
{
/* Make sure active replication slots are released */
if (MyReplicationSlot != NULL)
+ {
+ bool is_physical;
+ char *slotname;
+
+ is_physical = SlotIsPhysical(MyReplicationSlot);
+ slotname = pstrdup(NameStr(MyReplicationSlot->data.name));
ReplicationSlotRelease();
+ LogReplicationSlotReleased(is_physical, slotname);
+ pfree(slotname);
+ }
/* Also cleanup all the temporary slots. */
ReplicationSlotCleanup();
@@ -596,6 +605,42 @@ ReplicationSlotRelease(void)
LWLockRelease(ProcArrayLock);
}
+/*
+ * Helper function to log a message after replication slot acquisition.
+ */
+void
+LogReplicationSlotAquired(bool is_physical, char *slotname)
+{
+ Assert(MyReplicationSlot != NULL);
+
+ if (is_physical)
+ ereport(log_replication_commands ? LOG : DEBUG3,
+ (errmsg("acquired physical replication slot \"%s\"",
+ slotname)));
+ else
+ ereport(log_replication_commands ? LOG : DEBUG3,
+ (errmsg("acquired logical replication slot \"%s\"",
+ slotname)));
+}
+
+/*
+ * Helper function to log a message after replication slot release.
+ */
+void
+LogReplicationSlotReleased(bool is_physical, char *slotname)
+{
+ Assert(MyReplicationSlot == NULL);
+
+ if (is_physical)
+ ereport(log_replication_commands ? LOG : DEBUG3,
+ (errmsg("released physical replication slot \"%s\"",
+ slotname)));
+ else
+ ereport(log_replication_commands ? LOG : DEBUG3,
+ (errmsg("released logical replication slot \"%s\"",
+ slotname)));
+}
+
/*
* Cleanup all temporary slots created in current session.
*/
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75e8363e24..38d25ed3c8 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -322,7 +322,16 @@ WalSndErrorCleanup(void)
wal_segment_close(xlogreader);
if (MyReplicationSlot != NULL)
+ {
+ bool is_physical;
+ char *slotname;
+
+ is_physical = SlotIsPhysical(MyReplicationSlot);
+ slotname = pstrdup(NameStr(MyReplicationSlot->data.name));
ReplicationSlotRelease();
+ LogReplicationSlotReleased(is_physical, slotname);
+ pfree(slotname);
+ }
ReplicationSlotCleanup();
@@ -703,6 +712,8 @@ StartReplication(StartReplicationCmd *cmd)
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("cannot use a logical replication slot for physical replication")));
+ LogReplicationSlotAquired(true, cmd->slotname);
+
/*
* We don't need to verify the slot's restart_lsn here; instead we
* rely on the caller requesting the starting point to use. If the
@@ -843,7 +854,10 @@ StartReplication(StartReplicationCmd *cmd)
}
if (cmd->slotname)
+ {
ReplicationSlotRelease();
+ LogReplicationSlotReleased(true, cmd->slotname);
+ }
/*
* Copy is finished now. Send a single-row result set indicating the next
@@ -1260,6 +1274,8 @@ StartLogicalReplication(StartReplicationCmd *cmd)
cmd->slotname),
errdetail("This slot has been invalidated because it exceeded the maximum reserved size.")));
+ LogReplicationSlotAquired(false, cmd->slotname);
+
/*
* Force a disconnect, so that the decoding code doesn't need to care
* about an eventual switch from running in recovery, to running in a
@@ -1321,6 +1337,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
FreeDecodingContext(logical_decoding_ctx);
ReplicationSlotRelease();
+ LogReplicationSlotReleased(false, cmd->slotname);
replication_active = false;
if (got_STOPPING)
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1c0583fe26..ea41ccc7e2 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1203,7 +1203,7 @@ struct config_bool ConfigureNamesBool[] =
},
{
{"log_replication_commands", PGC_SUSET, LOGGING_WHAT,
- gettext_noop("Logs each replication command."),
+ gettext_noop("Logs each replication command and related activity."),
NULL
},
&log_replication_commands,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 8872c80cdf..147e4d4371 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -202,6 +202,8 @@ extern void ReplicationSlotDrop(const char *name, bool nowait);
extern void ReplicationSlotAcquire(const char *name, bool nowait);
extern void ReplicationSlotRelease(void);
+extern void LogReplicationSlotAquired(bool is_physical, char *slotname);
+extern void LogReplicationSlotReleased(bool is_physical, char *slotname);
extern void ReplicationSlotCleanup(void);
extern void ReplicationSlotSave(void);
extern void ReplicationSlotMarkDirty(void);
--
2.34.1
view thread (5+ 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]
Subject: Re: add log messages when replication slots become active and inactive (was Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?)
In-Reply-To: <CALj2ACXvqFcxro64q4gjDeBNLgyPH0E6bG-WJ6w7qBqbggj3=w@mail.gmail.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