[PATCH] Add option to create a replication slot in pg

public inbox for [email protected]  
help / color / mirror / Atom feed

[PATCH] Add option to create a replication slot in pg_basebackup if not yet present.
9+ messages / 5 participants
[nested] [flat]

* [PATCH] Add option to create a replication slot in pg_basebackup if not yet present.
@ 2017-08-18 09:28  Michael Banck <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Michael Banck @ 2017-08-18 09:28 UTC (permalink / raw)

When requesting a particular replication slot, the new pg_basebackup option
--create-slot tries to create it before starting to replicate from it in case
it does not exist already.

In order to allow reporting of temporary replication slot creation in --verbose
mode, further refactor the slot creation logic. Add new argument `is_temporary'
to CreateReplicationSlot() in streamutil.c which specifies whether a physical
replication slot is temporary or not. Update all callers. Move the creation of
the temporary replication slot for pg_basebackup from receivelog.c to
pg_basebackup.c and mention it in --verbose mode. At the same time, also create
the temporary slot via CreateReplicationSlot() instead of creating the
temporary slot with an explicit SQL command.
---
 doc/src/sgml/ref/pg_basebackup.sgml          | 12 ++++++
 src/bin/pg_basebackup/pg_basebackup.c        | 58 ++++++++++++++++++++++++++--
 src/bin/pg_basebackup/pg_receivewal.c        |  2 +-
 src/bin/pg_basebackup/pg_recvlogical.c       |  2 +-
 src/bin/pg_basebackup/receivelog.c           | 18 ---------
 src/bin/pg_basebackup/streamutil.c           | 18 ++++++---
 src/bin/pg_basebackup/streamutil.h           |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 25 ++++++++++--
 8 files changed, 103 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 2454d35af3..5397a185d2 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -269,6 +269,18 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>-C</option></term>
+      <term><option>--create-slot</option></term>
+      <listitem>
+       <para>
+        This option can only be used together with the <literal>--slot</literal> 
+        option.  It causes the specified slot name to be created if it does not
+        exist already.  
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-T <replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <term><option>--tablespace-mapping=<replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index dfb9b5ddcb..2bc7e868e0 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -92,6 +92,8 @@ static pg_time_t last_progress_report = 0;
 static int32 maxrate = 0;		/* no limit by default */
 static char *replication_slot = NULL;
 static bool temp_replication_slot = true;
+static bool create_slot = false;
+static bool no_slot = false;
 
 static bool success = false;
 static bool made_new_pgdata = false;
@@ -337,6 +339,7 @@ usage(void)
 			 "                         write recovery.conf for replication\n"));
 	printf(_("  -S, --slot=SLOTNAME    replication slot to use\n"));
 	printf(_("      --no-slot          prevent creation of temporary replication slot\n"));
+	printf(_("  -C, --create-slot      create replication slot if not present already\n"));
 	printf(_("  -T, --tablespace-mapping=OLDDIR=NEWDIR\n"
 			 "                         relocate tablespace in OLDDIR to NEWDIR\n"));
 	printf(_("  -X, --wal-method=none|fetch|stream\n"
@@ -492,8 +495,6 @@ LogStreamerMain(logstreamer_param *param)
 	stream.partial_suffix = NULL;
 	stream.replication_slot = replication_slot;
 	stream.temp_slot = param->temp_slot;
-	if (stream.temp_slot && !stream.replication_slot)
-		stream.replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn));
 
 	if (format == 'p')
 		stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync);
@@ -586,6 +587,29 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
 	else
 		param->temp_slot = temp_replication_slot;
 
+	/*
+	 * Create replication slot if one is needed.
+	 */
+	if (!no_slot && (temp_replication_slot || create_slot))
+	{
+		if (!replication_slot)
+			replication_slot = psprintf("pg_basebackup_%d", (int) getpid());
+
+		if (verbose)
+		{
+			if (temp_replication_slot)
+				fprintf(stderr, _("%s: creating temporary replication slot \"%s\"\n"),
+						progname, replication_slot);
+			else
+				fprintf(stderr, _("%s: creating replication slot \"%s\"\n"),
+						progname, replication_slot);
+		}
+
+		if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, true,
+								   temp_replication_slot, true))
+			disconnect_and_exit(1);
+	}
+
 	if (format == 'p')
 	{
 		/*
@@ -2099,12 +2123,12 @@ main(int argc, char **argv)
 		{"progress", no_argument, NULL, 'P'},
 		{"waldir", required_argument, NULL, 1},
 		{"no-slot", no_argument, NULL, 2},
+		{"create-slot", no_argument, NULL, 'C'},
 		{NULL, 0, NULL, 0}
 	};
 	int			c;
 
 	int			option_index;
-	bool		no_slot = false;
 
 	progname = get_progname(argv[0]);
 	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -2126,7 +2150,7 @@ main(int argc, char **argv)
 
 	atexit(cleanup_directories_atexit);
 
-	while ((c = getopt_long(argc, argv, "D:F:r:RT:X:l:nNzZ:d:c:h:p:U:s:S:wWvP",
+	while ((c = getopt_long(argc, argv, "D:F:r:RS:CT:X:l:nNzZ:d:c:h:p:U:s:wWvP",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -2162,6 +2186,9 @@ main(int argc, char **argv)
 				replication_slot = pg_strdup(optarg);
 				temp_replication_slot = false;
 				break;
+			case 'C':
+				create_slot = true;
+				break;
 			case 2:
 				no_slot = true;
 				break;
@@ -2347,6 +2374,29 @@ main(int argc, char **argv)
 		temp_replication_slot = false;
 	}
 
+	if (create_slot)
+	{
+		if (!replication_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot name\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+
+		if (no_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot, but --no-slot was requested\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+	}
+
 	if (strcmp(xlog_dir, "") != 0)
 	{
 		if (format != 'p')
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 4a1a5658fb..ce534e3290 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -701,7 +701,7 @@ main(int argc, char **argv)
 					progname, replication_slot);
 
 		if (!CreateReplicationSlot(conn, replication_slot, NULL, true,
-								   slot_exists_ok))
+								   false, slot_exists_ok))
 			disconnect_and_exit(1);
 		disconnect_and_exit(0);
 	}
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6811a55e76..8500f658ff 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -980,7 +980,7 @@ main(int argc, char **argv)
 					progname, replication_slot);
 
 		if (!CreateReplicationSlot(conn, replication_slot, plugin,
-								   false, slot_exists_ok))
+								   false, false, slot_exists_ok))
 			disconnect_and_exit(1);
 		startpos = InvalidXLogRecPtr;
 	}
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 888458f4a9..1f1e5e1268 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -522,24 +522,6 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 	}
 
 	/*
-	 * Create temporary replication slot if one is needed
-	 */
-	if (stream->temp_slot)
-	{
-		snprintf(query, sizeof(query),
-				 "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY PHYSICAL RESERVE_WAL",
-				 stream->replication_slot);
-		res = PQexec(conn, query);
-		if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		{
-			fprintf(stderr, _("%s: could not create temporary replication slot \"%s\": %s"),
-					progname, stream->replication_slot, PQerrorMessage(conn));
-			PQclear(res);
-			return false;
-		}
-	}
-
-	/*
 	 * initialize flush position to starting point, it's the caller's
 	 * responsibility that that's sane.
 	 */
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 9d40744a34..e4182f4f48 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -322,7 +322,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
  */
 bool
 CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
-					  bool is_physical, bool slot_exists_ok)
+					  bool is_physical, bool is_temporary, bool slot_exists_ok)
 {
 	PQExpBuffer query;
 	PGresult   *res;
@@ -335,12 +335,20 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 
 	/* Build query */
 	if (is_physical)
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" PHYSICAL",
-						  slot_name);
+		if (is_temporary)
+			appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY PHYSICAL",
+							  slot_name);
+		else
+			appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" PHYSICAL",
+							  slot_name);
 	else
 	{
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" LOGICAL \"%s\"",
-						  slot_name, plugin);
+		if (is_temporary)
+			appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY LOGICAL \"%s\"",
+							  slot_name, plugin);
+		else
+			appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" LOGICAL \"%s\"",
+							  slot_name, plugin);
 		if (PQserverVersion(conn) >= 100000)
 			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
 			appendPQExpBuffer(query, " NOEXPORT_SNAPSHOT");
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 6f6878679f..1dffb55b91 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -32,7 +32,7 @@ extern PGconn *GetConnection(void);
 
 /* Replication commands */
 extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
-					  const char *plugin, bool is_physical,
+					  const char *plugin, bool is_physical, bool is_temporary,
 					  bool slot_exists_ok);
 extern bool DropReplicationSlot(PGconn *conn, const char *slot_name);
 extern bool RunIdentifySystem(PGconn *conn, char **sysid,
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index a00f7b0e1a..1e4b7d8b21 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@ use Cwd;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 72;
+use Test::More tests => 76;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -256,18 +256,35 @@ $node->command_ok(
 	'pg_basebackup -X stream runs with --no-slot');
 
 $node->command_fails(
-	[ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
+	[ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ],
 	'pg_basebackup with replication slot fails without -X stream');
 $node->command_fails(
 	[   'pg_basebackup',             '-D',
 		"$tempdir/backupxs_sl_fail", '-X',
 		'stream',                    '-S',
-		'slot1' ],
+		'slot0' ],
 	'pg_basebackup fails with nonexistent replication slot');
 
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C' ],
+	'pg_basebackup -C fails without slot name');
+
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0', '--no-slot' ],
+	'pg_basebackup fails with -C -S --no-slot');
+
+$node->command_ok(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0' ],
+	'pg_basebackup -C -S creates previously nonexistent replication slot');
+
+my $lsn = $node->safe_psql('postgres',
+	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot0'}
+);
+like($lsn, qr!^0/[0-9A-F]{7,8}$!, 'slot is present');
+
 $node->safe_psql('postgres',
 	q{SELECT * FROM pg_create_physical_replication_slot('slot1')});
-my $lsn = $node->safe_psql('postgres',
+$lsn = $node->safe_psql('postgres',
 	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'}
 );
 is($lsn, '', 'restart LSN of new slot is null');
-- 
2.11.0


--vkogqOf2sHV7VnPd
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--vkogqOf2sHV7VnPd--




^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* [PATCH] Add option to create a replication slot in pg_basebackup if not yet present.
@ 2017-09-10 16:07  Michael Banck <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Michael Banck @ 2017-09-10 16:07 UTC (permalink / raw)

When requesting a particular replication slot, the new pg_basebackup option
--create-slot tries to create it before starting to replicate from it unless
it exists already.

In order to allow reporting of temporary replication slot creation in --verbose
mode, further refactor the slot creation logic. Add new argument `is_temporary'
to CreateReplicationSlot() in streamutil.c which specifies whether a physical
replication slot is temporary or not. Update all callers. Move the creation of
the temporary replication slot for pg_basebackup from receivelog.c to
pg_basebackup.c and mention it in --verbose mode. At the same time, also create
the temporary slot via CreateReplicationSlot() instead of creating the
temporary slot with an explicit SQL command.
---
 doc/src/sgml/ref/pg_basebackup.sgml          | 12 ++++++
 src/bin/pg_basebackup/pg_basebackup.c        | 56 ++++++++++++++++++++++++++--
 src/bin/pg_basebackup/pg_receivewal.c        |  2 +-
 src/bin/pg_basebackup/pg_recvlogical.c       |  2 +-
 src/bin/pg_basebackup/receivelog.c           | 18 ---------
 src/bin/pg_basebackup/streamutil.c           | 15 +++++---
 src/bin/pg_basebackup/streamutil.h           |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 34 +++++++++++++++--
 8 files changed, 107 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 2454d35af3..57086295d3 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -269,6 +269,18 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>-C</option></term>
+      <term><option>--create-slot</option></term>
+      <listitem>
+       <para>
+        This option can only be used together with the <literal>--slot</literal>
+        option.  It causes the specified slot name to be created unless it
+        exists already.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-T <replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <term><option>--tablespace-mapping=<replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 51509d150e..35ea707384 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -92,6 +92,8 @@ static pg_time_t last_progress_report = 0;
 static int32 maxrate = 0;		/* no limit by default */
 static char *replication_slot = NULL;
 static bool temp_replication_slot = true;
+static bool create_slot = false;
+static bool no_slot = false;
 
 static bool success = false;
 static bool made_new_pgdata = false;
@@ -337,6 +339,7 @@ usage(void)
 			 "                         write recovery.conf for replication\n"));
 	printf(_("  -S, --slot=SLOTNAME    replication slot to use\n"));
 	printf(_("      --no-slot          prevent creation of temporary replication slot\n"));
+	printf(_("  -C, --create-slot      create replication slot if not present already\n"));
 	printf(_("  -T, --tablespace-mapping=OLDDIR=NEWDIR\n"
 			 "                         relocate tablespace in OLDDIR to NEWDIR\n"));
 	printf(_("  -X, --wal-method=none|fetch|stream\n"
@@ -492,8 +495,6 @@ LogStreamerMain(logstreamer_param *param)
 	stream.partial_suffix = NULL;
 	stream.replication_slot = replication_slot;
 	stream.temp_slot = param->temp_slot;
-	if (stream.temp_slot && !stream.replication_slot)
-		stream.replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn));
 
 	if (format == 'p')
 		stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync);
@@ -586,6 +587,27 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
 	else
 		param->temp_slot = temp_replication_slot;
 
+	/*
+	 * Create replication slot if one is needed.
+	 */
+	if (!no_slot && (temp_replication_slot || create_slot))
+	{
+		if (!replication_slot)
+			replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn));
+
+		if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, true,
+								   temp_replication_slot, false))
+			disconnect_and_exit(1);
+
+		if (verbose)
+			if (temp_replication_slot)
+				fprintf(stderr, _("%s: temporary replication slot \"%s\" created\n"),
+						progname, replication_slot);
+			else
+				fprintf(stderr, _("%s: replication slot \"%s\" created\n"),
+						progname, replication_slot);
+	}
+
 	if (format == 'p')
 	{
 		/*
@@ -2099,12 +2121,12 @@ main(int argc, char **argv)
 		{"progress", no_argument, NULL, 'P'},
 		{"waldir", required_argument, NULL, 1},
 		{"no-slot", no_argument, NULL, 2},
+		{"create-slot", no_argument, NULL, 'C'},
 		{NULL, 0, NULL, 0}
 	};
 	int			c;
 
 	int			option_index;
-	bool		no_slot = false;
 
 	progname = get_progname(argv[0]);
 	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -2126,7 +2148,7 @@ main(int argc, char **argv)
 
 	atexit(cleanup_directories_atexit);
 
-	while ((c = getopt_long(argc, argv, "D:F:r:RT:X:l:nNzZ:d:c:h:p:U:s:S:wWvP",
+	while ((c = getopt_long(argc, argv, "D:F:r:RS:CT:X:l:nNzZ:d:c:h:p:U:s:wWvP",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -2162,6 +2184,9 @@ main(int argc, char **argv)
 				replication_slot = pg_strdup(optarg);
 				temp_replication_slot = false;
 				break;
+			case 'C':
+				create_slot = true;
+				break;
 			case 2:
 				no_slot = true;
 				break;
@@ -2347,6 +2372,29 @@ main(int argc, char **argv)
 		temp_replication_slot = false;
 	}
 
+	if (create_slot)
+	{
+		if (!replication_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot name\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+
+		if (no_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot, but --no-slot was requested\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+	}
+
 	if (xlog_dir)
 	{
 		if (format != 'p')
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 4a1a5658fb..8dfe7fd549 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -700,7 +700,7 @@ main(int argc, char **argv)
 					_("%s: creating replication slot \"%s\"\n"),
 					progname, replication_slot);
 
-		if (!CreateReplicationSlot(conn, replication_slot, NULL, true,
+		if (!CreateReplicationSlot(conn, replication_slot, NULL, true, false,
 								   slot_exists_ok))
 			disconnect_and_exit(1);
 		disconnect_and_exit(0);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6811a55e76..6499f58e3c 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -979,7 +979,7 @@ main(int argc, char **argv)
 					_("%s: creating replication slot \"%s\"\n"),
 					progname, replication_slot);
 
-		if (!CreateReplicationSlot(conn, replication_slot, plugin,
+		if (!CreateReplicationSlot(conn, replication_slot, plugin, false,
 								   false, slot_exists_ok))
 			disconnect_and_exit(1);
 		startpos = InvalidXLogRecPtr;
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 888458f4a9..1f1e5e1268 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -522,24 +522,6 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 	}
 
 	/*
-	 * Create temporary replication slot if one is needed
-	 */
-	if (stream->temp_slot)
-	{
-		snprintf(query, sizeof(query),
-				 "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY PHYSICAL RESERVE_WAL",
-				 stream->replication_slot);
-		res = PQexec(conn, query);
-		if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		{
-			fprintf(stderr, _("%s: could not create temporary replication slot \"%s\": %s"),
-					progname, stream->replication_slot, PQerrorMessage(conn));
-			PQclear(res);
-			return false;
-		}
-	}
-
-	/*
 	 * initialize flush position to starting point, it's the caller's
 	 * responsibility that that's sane.
 	 */
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 9d40744a34..be3a9786fd 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -322,7 +322,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
  */
 bool
 CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
-					  bool is_physical, bool slot_exists_ok)
+					  bool is_physical, bool is_temporary, bool slot_exists_ok)
 {
 	PQExpBuffer query;
 	PGresult   *res;
@@ -334,13 +334,18 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	Assert(slot_name != NULL);
 
 	/* Build query */
+	appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
 	if (is_physical)
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" PHYSICAL",
-						  slot_name);
+	{
+		if (is_temporary)
+			appendPQExpBuffer(query, " TEMPORARY");
+		appendPQExpBuffer(query, " PHYSICAL RESERVE_WAL");
+	}
 	else
 	{
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" LOGICAL \"%s\"",
-						  slot_name, plugin);
+		if (is_temporary)
+			appendPQExpBuffer(query, " TEMPORARY");
+		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
 		if (PQserverVersion(conn) >= 100000)
 			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
 			appendPQExpBuffer(query, " NOEXPORT_SNAPSHOT");
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 6f6878679f..1dffb55b91 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -32,7 +32,7 @@ extern PGconn *GetConnection(void);
 
 /* Replication commands */
 extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
-					  const char *plugin, bool is_physical,
+					  const char *plugin, bool is_physical, bool is_temporary,
 					  bool slot_exists_ok);
 extern bool DropReplicationSlot(PGconn *conn, const char *slot_name);
 extern bool RunIdentifySystem(PGconn *conn, char **sysid,
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index a00f7b0e1a..e287645745 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@ use Cwd;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 72;
+use Test::More tests => 78;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -256,18 +256,44 @@ $node->command_ok(
 	'pg_basebackup -X stream runs with --no-slot');
 
 $node->command_fails(
-	[ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
+	[ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ],
 	'pg_basebackup with replication slot fails without -X stream');
 $node->command_fails(
 	[   'pg_basebackup',             '-D',
 		"$tempdir/backupxs_sl_fail", '-X',
 		'stream',                    '-S',
-		'slot1' ],
+		'slot0' ],
 	'pg_basebackup fails with nonexistent replication slot');
 
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C' ],
+	'pg_basebackup -C fails without slot name');
+
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0', '--no-slot' ],
+	'pg_basebackup fails with -C -S --no-slot');
+
+$node->command_ok(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0' ],
+	'pg_basebackup -C -S creates previously nonexistent replication slot');
+
+my $lsn = $node->safe_psql('postgres',
+	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot0'}
+);
+like($lsn, qr!^0/[0-9A-F]{7,8}$!, 'replication slot is present');
+
+my $checkpoint_lsn = $node->safe_psql('postgres',
+	q{SELECT checkpoint_lsn FROM pg_control_checkpoint(), pg_replication_slots WHERE slot_name = 'slot0' AND restart_lsn <= pg_control_checkpoint.checkpoint_lsn}
+);
+like($lsn, qr!^0/[0-9A-F]{7,8}$!, "replication slot LSN $lsn is lower than latest checkpoint LSN $checkpoint_lsn");
+
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot1", '-C', '-S', 'slot0' ],
+	'pg_basebackup fails with -C -S and a previously existing slot');
+
 $node->safe_psql('postgres',
 	q{SELECT * FROM pg_create_physical_replication_slot('slot1')});
-my $lsn = $node->safe_psql('postgres',
+$lsn = $node->safe_psql('postgres',
 	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'}
 );
 is($lsn, '', 'restart LSN of new slot is null');
-- 
2.11.0


--Y7xTucakfITjPcLV
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--Y7xTucakfITjPcLV--




^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* [PATCH] Add option to create a replication slot in pg_basebackup if not yet present.
@ 2017-09-10 16:07  Michael Banck <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Michael Banck @ 2017-09-10 16:07 UTC (permalink / raw)

When requesting a particular replication slot, the new pg_basebackup option
--create-slot tries to create it before starting to replicate from it unless
it exists already.

In order to allow reporting of temporary replication slot creation in --verbose
mode, further refactor the slot creation logic. Add new argument `is_temporary'
to CreateReplicationSlot() in streamutil.c which specifies whether a physical
replication slot is temporary or not. Update all callers. Move the creation of
the temporary replication slot for pg_basebackup from receivelog.c to
pg_basebackup.c and mention it in --verbose mode. At the same time, also create
the temporary slot via CreateReplicationSlot() instead of creating the
temporary slot with an explicit SQL command.
---
 doc/src/sgml/ref/pg_basebackup.sgml          | 12 ++++++
 src/bin/pg_basebackup/pg_basebackup.c        | 56 ++++++++++++++++++++++++++--
 src/bin/pg_basebackup/pg_receivewal.c        |  2 +-
 src/bin/pg_basebackup/pg_recvlogical.c       |  2 +-
 src/bin/pg_basebackup/receivelog.c           | 18 ---------
 src/bin/pg_basebackup/streamutil.c           | 15 +++++---
 src/bin/pg_basebackup/streamutil.h           |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 34 +++++++++++++++--
 8 files changed, 107 insertions(+), 34 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 2454d35af3..57086295d3 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -269,6 +269,18 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
+      <term><option>-C</option></term>
+      <term><option>--create-slot</option></term>
+      <listitem>
+       <para>
+        This option can only be used together with the <literal>--slot</literal>
+        option.  It causes the specified slot name to be created unless it
+        exists already.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
       <term><option>-T <replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <term><option>--tablespace-mapping=<replaceable class="parameter">olddir</replaceable>=<replaceable class="parameter">newdir</replaceable></option></term>
       <listitem>
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 51509d150e..35ea707384 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -92,6 +92,8 @@ static pg_time_t last_progress_report = 0;
 static int32 maxrate = 0;		/* no limit by default */
 static char *replication_slot = NULL;
 static bool temp_replication_slot = true;
+static bool create_slot = false;
+static bool no_slot = false;
 
 static bool success = false;
 static bool made_new_pgdata = false;
@@ -337,6 +339,7 @@ usage(void)
 			 "                         write recovery.conf for replication\n"));
 	printf(_("  -S, --slot=SLOTNAME    replication slot to use\n"));
 	printf(_("      --no-slot          prevent creation of temporary replication slot\n"));
+	printf(_("  -C, --create-slot      create replication slot if not present already\n"));
 	printf(_("  -T, --tablespace-mapping=OLDDIR=NEWDIR\n"
 			 "                         relocate tablespace in OLDDIR to NEWDIR\n"));
 	printf(_("  -X, --wal-method=none|fetch|stream\n"
@@ -492,8 +495,6 @@ LogStreamerMain(logstreamer_param *param)
 	stream.partial_suffix = NULL;
 	stream.replication_slot = replication_slot;
 	stream.temp_slot = param->temp_slot;
-	if (stream.temp_slot && !stream.replication_slot)
-		stream.replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn));
 
 	if (format == 'p')
 		stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync);
@@ -586,6 +587,27 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier)
 	else
 		param->temp_slot = temp_replication_slot;
 
+	/*
+	 * Create replication slot if one is needed.
+	 */
+	if (!no_slot && (temp_replication_slot || create_slot))
+	{
+		if (!replication_slot)
+			replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn));
+
+		if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, true,
+								   temp_replication_slot, false))
+			disconnect_and_exit(1);
+
+		if (verbose)
+			if (temp_replication_slot)
+				fprintf(stderr, _("%s: temporary replication slot \"%s\" created\n"),
+						progname, replication_slot);
+			else
+				fprintf(stderr, _("%s: replication slot \"%s\" created\n"),
+						progname, replication_slot);
+	}
+
 	if (format == 'p')
 	{
 		/*
@@ -2099,12 +2121,12 @@ main(int argc, char **argv)
 		{"progress", no_argument, NULL, 'P'},
 		{"waldir", required_argument, NULL, 1},
 		{"no-slot", no_argument, NULL, 2},
+		{"create-slot", no_argument, NULL, 'C'},
 		{NULL, 0, NULL, 0}
 	};
 	int			c;
 
 	int			option_index;
-	bool		no_slot = false;
 
 	progname = get_progname(argv[0]);
 	set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_basebackup"));
@@ -2126,7 +2148,7 @@ main(int argc, char **argv)
 
 	atexit(cleanup_directories_atexit);
 
-	while ((c = getopt_long(argc, argv, "D:F:r:RT:X:l:nNzZ:d:c:h:p:U:s:S:wWvP",
+	while ((c = getopt_long(argc, argv, "D:F:r:RS:CT:X:l:nNzZ:d:c:h:p:U:s:wWvP",
 							long_options, &option_index)) != -1)
 	{
 		switch (c)
@@ -2162,6 +2184,9 @@ main(int argc, char **argv)
 				replication_slot = pg_strdup(optarg);
 				temp_replication_slot = false;
 				break;
+			case 'C':
+				create_slot = true;
+				break;
 			case 2:
 				no_slot = true;
 				break;
@@ -2347,6 +2372,29 @@ main(int argc, char **argv)
 		temp_replication_slot = false;
 	}
 
+	if (create_slot)
+	{
+		if (!replication_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot name\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+
+		if (no_slot)
+		{
+			fprintf(stderr,
+					_("%s: creation of replication slots requires a slot, but --no-slot was requested\n"),
+					progname);
+			fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+					progname);
+			exit(1);
+		}
+	}
+
 	if (xlog_dir)
 	{
 		if (format != 'p')
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 4a1a5658fb..8dfe7fd549 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -700,7 +700,7 @@ main(int argc, char **argv)
 					_("%s: creating replication slot \"%s\"\n"),
 					progname, replication_slot);
 
-		if (!CreateReplicationSlot(conn, replication_slot, NULL, true,
+		if (!CreateReplicationSlot(conn, replication_slot, NULL, true, false,
 								   slot_exists_ok))
 			disconnect_and_exit(1);
 		disconnect_and_exit(0);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 6811a55e76..6499f58e3c 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -979,7 +979,7 @@ main(int argc, char **argv)
 					_("%s: creating replication slot \"%s\"\n"),
 					progname, replication_slot);
 
-		if (!CreateReplicationSlot(conn, replication_slot, plugin,
+		if (!CreateReplicationSlot(conn, replication_slot, plugin, false,
 								   false, slot_exists_ok))
 			disconnect_and_exit(1);
 		startpos = InvalidXLogRecPtr;
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 888458f4a9..1f1e5e1268 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -522,24 +522,6 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
 	}
 
 	/*
-	 * Create temporary replication slot if one is needed
-	 */
-	if (stream->temp_slot)
-	{
-		snprintf(query, sizeof(query),
-				 "CREATE_REPLICATION_SLOT \"%s\" TEMPORARY PHYSICAL RESERVE_WAL",
-				 stream->replication_slot);
-		res = PQexec(conn, query);
-		if (PQresultStatus(res) != PGRES_TUPLES_OK)
-		{
-			fprintf(stderr, _("%s: could not create temporary replication slot \"%s\": %s"),
-					progname, stream->replication_slot, PQerrorMessage(conn));
-			PQclear(res);
-			return false;
-		}
-	}
-
-	/*
 	 * initialize flush position to starting point, it's the caller's
 	 * responsibility that that's sane.
 	 */
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 9d40744a34..be3a9786fd 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -322,7 +322,7 @@ RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
  */
 bool
 CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
-					  bool is_physical, bool slot_exists_ok)
+					  bool is_physical, bool is_temporary, bool slot_exists_ok)
 {
 	PQExpBuffer query;
 	PGresult   *res;
@@ -334,13 +334,18 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	Assert(slot_name != NULL);
 
 	/* Build query */
+	appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
 	if (is_physical)
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" PHYSICAL",
-						  slot_name);
+	{
+		if (is_temporary)
+			appendPQExpBuffer(query, " TEMPORARY");
+		appendPQExpBuffer(query, " PHYSICAL RESERVE_WAL");
+	}
 	else
 	{
-		appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\" LOGICAL \"%s\"",
-						  slot_name, plugin);
+		if (is_temporary)
+			appendPQExpBuffer(query, " TEMPORARY");
+		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
 		if (PQserverVersion(conn) >= 100000)
 			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
 			appendPQExpBuffer(query, " NOEXPORT_SNAPSHOT");
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 6f6878679f..1dffb55b91 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -32,7 +32,7 @@ extern PGconn *GetConnection(void);
 
 /* Replication commands */
 extern bool CreateReplicationSlot(PGconn *conn, const char *slot_name,
-					  const char *plugin, bool is_physical,
+					  const char *plugin, bool is_physical, bool is_temporary,
 					  bool slot_exists_ok);
 extern bool DropReplicationSlot(PGconn *conn, const char *slot_name);
 extern bool RunIdentifySystem(PGconn *conn, char **sysid,
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index a00f7b0e1a..e287645745 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -4,7 +4,7 @@ use Cwd;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 72;
+use Test::More tests => 78;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -256,18 +256,44 @@ $node->command_ok(
 	'pg_basebackup -X stream runs with --no-slot');
 
 $node->command_fails(
-	[ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ],
+	[ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ],
 	'pg_basebackup with replication slot fails without -X stream');
 $node->command_fails(
 	[   'pg_basebackup',             '-D',
 		"$tempdir/backupxs_sl_fail", '-X',
 		'stream',                    '-S',
-		'slot1' ],
+		'slot0' ],
 	'pg_basebackup fails with nonexistent replication slot');
 
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C' ],
+	'pg_basebackup -C fails without slot name');
+
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0', '--no-slot' ],
+	'pg_basebackup fails with -C -S --no-slot');
+
+$node->command_ok(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot", '-C', '-S', 'slot0' ],
+	'pg_basebackup -C -S creates previously nonexistent replication slot');
+
+my $lsn = $node->safe_psql('postgres',
+	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot0'}
+);
+like($lsn, qr!^0/[0-9A-F]{7,8}$!, 'replication slot is present');
+
+my $checkpoint_lsn = $node->safe_psql('postgres',
+	q{SELECT checkpoint_lsn FROM pg_control_checkpoint(), pg_replication_slots WHERE slot_name = 'slot0' AND restart_lsn <= pg_control_checkpoint.checkpoint_lsn}
+);
+like($lsn, qr!^0/[0-9A-F]{7,8}$!, "replication slot LSN $lsn is lower than latest checkpoint LSN $checkpoint_lsn");
+
+$node->command_fails(
+	[   'pg_basebackup', '-D', "$tempdir/backupxs_slot1", '-C', '-S', 'slot0' ],
+	'pg_basebackup fails with -C -S and a previously existing slot');
+
 $node->safe_psql('postgres',
 	q{SELECT * FROM pg_create_physical_replication_slot('slot1')});
-my $lsn = $node->safe_psql('postgres',
+$lsn = $node->safe_psql('postgres',
 	q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'slot1'}
 );
 is($lsn, '', 'restart LSN of new slot is null');
-- 
2.11.0


--ibTvN161/egqYuK8
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--ibTvN161/egqYuK8--




^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* [PATCH v6 1/3] CatCache expiration feature
@ 2020-11-18 07:54  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Kyotaro Horiguchi @ 2020-11-18 07:54 UTC (permalink / raw)

---
 src/backend/access/transam/xact.c  |  3 ++
 src/backend/utils/cache/catcache.c | 87 +++++++++++++++++++++++++++++-
 src/backend/utils/misc/guc.c       | 12 +++++
 src/include/utils/catcache.h       | 19 +++++++
 4 files changed, 120 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index a2068e3fd4..86888d2409 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1086,6 +1086,9 @@ static void
 AtStart_Cache(void)
 {
 	AcceptInvalidationMessages();
+
+	if (xactStartTimestamp != 0)
+		SetCatCacheClock(xactStartTimestamp);
 }
 
 /*
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index fa2b49c676..644d92dd9a 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -38,6 +38,7 @@
 #include "utils/rel.h"
 #include "utils/resowner_private.h"
 #include "utils/syscache.h"
+#include "utils/timestamp.h"
 
 
  /* #define CACHEDEBUG */	/* turns DEBUG elogs on */
@@ -60,9 +61,19 @@
 #define CACHE_elog(...)
 #endif
 
+/*
+ * GUC variable to define the minimum age of entries that will be considered
+ * to be evicted in seconds. -1 to disable the feature.
+ */
+int catalog_cache_prune_min_age = -1;
+uint64	prune_min_age_us;
+
 /* Cache management header --- pointer is NULL until created */
 static CatCacheHeader *CacheHdr = NULL;
 
+/* Clock for the last accessed time of a catcache entry. */
+uint64	catcacheclock = 0;
+
 static inline HeapTuple SearchCatCacheInternal(CatCache *cache,
 											   int nkeys,
 											   Datum v1, Datum v2,
@@ -74,6 +85,7 @@ static pg_noinline HeapTuple SearchCatCacheMiss(CatCache *cache,
 												Index hashIndex,
 												Datum v1, Datum v2,
 												Datum v3, Datum v4);
+static bool CatCacheCleanupOldEntries(CatCache *cp);
 
 static uint32 CatalogCacheComputeHashValue(CatCache *cache, int nkeys,
 										   Datum v1, Datum v2, Datum v3, Datum v4);
@@ -99,6 +111,15 @@ static void CatCacheFreeKeys(TupleDesc tupdesc, int nkeys, int *attnos,
 static void CatCacheCopyKeys(TupleDesc tupdesc, int nkeys, int *attnos,
 							 Datum *srckeys, Datum *dstkeys);
 
+/* GUC assign function */
+void
+assign_catalog_cache_prune_min_age(int newval, void *extra)
+{
+	if (newval < 0)
+		prune_min_age_us = UINT64_MAX;
+	else
+		prune_min_age_us = ((uint64) newval) * USECS_PER_SEC;
+}
 
 /*
  *					internal support functions
@@ -1264,6 +1285,9 @@ SearchCatCacheInternal(CatCache *cache,
 		 */
 		dlist_move_head(bucket, &ct->cache_elem);
 
+		/* Record the last access timestamp */
+		ct->lastaccess = catcacheclock;
+
 		/*
 		 * If it's a positive entry, bump its refcount and return it. If it's
 		 * negative, we can report failure to the caller.
@@ -1425,6 +1449,61 @@ SearchCatCacheMiss(CatCache *cache,
 	return &ct->tuple;
 }
 
+/*
+ * CatCacheCleanupOldEntries - Remove infrequently-used entries
+ *
+ * Catcache entries happen to be left unused for a long time for several
+ * reasons. Remove such entries to prevent catcache from bloating. It is based
+ * on the similar algorithm with buffer eviction. Entries that are accessed
+ * several times in a certain period live longer than those that have had less
+ * access in the same duration.
+ */
+static bool
+CatCacheCleanupOldEntries(CatCache *cp)
+{
+	int		nremoved = 0;
+	int		i;
+	long	oldest_ts = catcacheclock;
+	uint64	prune_threshold = catcacheclock - prune_min_age_us;
+
+	/* Scan over the whole hash to find entries to remove */
+	for (i = 0 ; i < cp->cc_nbuckets ; i++)
+	{
+		dlist_mutable_iter	iter;
+
+		dlist_foreach_modify(iter, &cp->cc_bucket[i])
+		{
+			CatCTup    *ct = dlist_container(CatCTup, cache_elem, iter.cur);
+
+			/* Don't remove referenced entries */
+			if (ct->refcount == 0 &&
+				(ct->c_list == NULL || ct->c_list->refcount == 0))
+			{
+				if (ct->lastaccess < prune_threshold)
+				{
+					CatCacheRemoveCTup(cp, ct);
+					nremoved++;
+
+					/* don't let the removed entry update oldest_ts */
+					continue;
+				}
+			}
+
+			/* update the oldest timestamp if the entry remains alive */
+			if (ct->lastaccess < oldest_ts)
+				oldest_ts = ct->lastaccess;
+		}
+	}
+
+	cp->cc_oldest_ts = oldest_ts;
+
+	if (nremoved > 0)
+		elog(DEBUG1, "pruning catalog cache id=%d for %s: removed %d / %d",
+			 cp->id, cp->cc_relname, nremoved, cp->cc_ntup + nremoved);
+
+	return nremoved > 0;
+}
+
 /*
  *	ReleaseCatCache
  *
@@ -1888,6 +1967,7 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
 	ct->dead = false;
 	ct->negative = negative;
 	ct->hash_value = hashValue;
+	ct->lastaccess = catcacheclock;
 
 	dlist_push_head(&cache->cc_bucket[hashIndex], &ct->cache_elem);
 
@@ -1899,7 +1979,12 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments,
 	 * arbitrarily, we enlarge when fill factor > 2.
 	 */
 	if (cache->cc_ntup > cache->cc_nbuckets * 2)
-		RehashCatCache(cache);
+	{
+		/* try removing old entries before expanding hash */
+		if (catcacheclock - cache->cc_oldest_ts < prune_min_age_us ||
+			!CatCacheCleanupOldEntries(cache))
+			RehashCatCache(cache);
+	}
 
 	return ct;
 }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 17579eeaca..255e9fa73d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -88,6 +88,7 @@
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/bytea.h"
+#include "utils/catcache.h"
 #include "utils/float.h"
 #include "utils/guc_tables.h"
 #include "utils/memutils.h"
@@ -3445,6 +3446,17 @@ static struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"catalog_cache_prune_min_age", PGC_USERSET, RESOURCES_MEM,
+			gettext_noop("System catalog cache entries that are living unused more than this seconds are considered for removal."),
+			gettext_noop("The value of -1 turns off pruning."),
+			GUC_UNIT_S
+		},
+		&catalog_cache_prune_min_age,
+		-1, -1, INT_MAX,
+		NULL, assign_catalog_cache_prune_min_age, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h
index ddc2762eb3..291e857e38 100644
--- a/src/include/utils/catcache.h
+++ b/src/include/utils/catcache.h
@@ -22,6 +22,7 @@
 
 #include "access/htup.h"
 #include "access/skey.h"
+#include "datatype/timestamp.h"
 #include "lib/ilist.h"
 #include "utils/relcache.h"
 
@@ -61,6 +62,7 @@ typedef struct catcache
 	slist_node	cc_next;		/* list link */
 	ScanKeyData cc_skey[CATCACHE_MAXKEYS];	/* precomputed key info for heap
 											 * scans */
+	uint64		cc_oldest_ts;	/* timestamp (us) of the oldest tuple */
 
 	/*
 	 * Keep these at the end, so that compiling catcache.c with CATCACHE_STATS
@@ -119,6 +121,7 @@ typedef struct catctup
 	bool		dead;			/* dead but not yet removed? */
 	bool		negative;		/* negative cache entry? */
 	HeapTupleData tuple;		/* tuple management header */
+	uint64		lastaccess;		/* timestamp in us of the last usage */
 
 	/*
 	 * The tuple may also be a member of at most one CatCList.  (If a single
@@ -189,6 +192,22 @@ typedef struct catcacheheader
 /* this extern duplicates utils/memutils.h... */
 extern PGDLLIMPORT MemoryContext CacheMemoryContext;
 
+
+/* for guc.c, not PGDLLPMPORT'ed */
+extern int catalog_cache_prune_min_age;
+
+/* source clock for access timestamp of catcache entries */
+extern uint64 catcacheclock;
+
+/* SetCatCacheClock - set catcache timestamp source clock */
+static inline void
+SetCatCacheClock(TimestampTz ts)
+{
+	catcacheclock = (uint64) ts;
+}
+
+extern void assign_catalog_cache_prune_min_age(int newval, void *extra);
+
 extern void CreateCacheMemoryContext(void);
 
 extern CatCache *InitCatCache(int id, Oid reloid, Oid indexoid,
-- 
2.27.0


----Next_Part(Thu_Jan_14_17_32_27_2021_995)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v6-0002-Remove-dead-flag-from-catcache-tuple.patch"



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: How to send patch with so many files changes?
@ 2024-09-25 00:36  Tony Wayne <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Tony Wayne @ 2024-09-25 00:36 UTC (permalink / raw)
  To: pgsql-hackers

On Wed, Sep 25, 2024 at 6:02 AM Tony Wayne <[email protected]>
wrote:

>
> I am new in contributing to postgres. I have a doubt regarding if we want
> to send a patch which has an extension and also has changes in pg source
> also,what's the way to do it?
>
> is git diff enough?

^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: How to send patch with so many files changes?
@ 2024-09-25 00:38  David G. Johnston <[email protected]>
  parent: Tony Wayne <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: David G. Johnston @ 2024-09-25 00:38 UTC (permalink / raw)
  To: Tony Wayne <[email protected]>; +Cc: pgsql-hackers

On Tue, Sep 24, 2024 at 5:37 PM Tony Wayne <[email protected]>
wrote:

>
> On Wed, Sep 25, 2024 at 6:02 AM Tony Wayne <[email protected]>
> wrote:
>
>>
>> I am new in contributing to postgres. I have a doubt regarding if we want
>> to send a patch which has an extension and also has changes in pg source
>> also,what's the way to do it?
>>
>> is git diff enough?
>

Usually you'd want to use format-patch so your commit message(s) make it
into the artifact.  Especially for something complex/large.

David J.

^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: How to send patch with so many files changes?
@ 2024-09-25 00:49  Tony Wayne <[email protected]>
  parent: David G. Johnston <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Tony Wayne @ 2024-09-25 00:49 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; pgsql-hackers

These changes are for core ,I think it would be better to either move whole
changes to core or contrib as an extension.

On Wed, Sep 25, 2024 at 6:09 AM David G. Johnston <
[email protected]> wrote:

> On Tue, Sep 24, 2024 at 5:37 PM Tony Wayne <[email protected]>
> wrote:
>
>>
>> On Wed, Sep 25, 2024 at 6:02 AM Tony Wayne <[email protected]>
>> wrote:
>>
>>>
>>> I am new in contributing to postgres. I have a doubt regarding if we
>>> want to send a patch which has an extension and also has changes in pg
>>> source also,what's the way to do it?
>>>
>>> is git diff enough?
>>
>
> Usually you'd want to use format-patch so your commit message(s) make it
> into the artifact.  Especially for something complex/large.
>
> David J.
>

^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: How to send patch with so many files changes?
@ 2024-09-25 01:06  Michael Paquier <[email protected]>
  parent: Tony Wayne <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Michael Paquier @ 2024-09-25 01:06 UTC (permalink / raw)
  To: Tony Wayne <[email protected]>; +Cc: David G. Johnston <[email protected]>; pgsql-hackers

On Wed, Sep 25, 2024 at 06:19:39AM +0530, Tony Wayne wrote:
> These changes are for core ,I think it would be better to either move whole
> changes to core or contrib as an extension.

Please avoid top-posting.  The community mailing lists use
bottom-posting, to ease discussions.  See:
https://en.wikipedia.org/wiki/Posting_style#Bottom-posting

> On Wed, Sep 25, 2024 at 6:09 AM David G. Johnston <
> [email protected]> wrote:
>> Usually you'd want to use format-patch so your commit message(s) make it
>> into the artifact.  Especially for something complex/large.

The community wiki has some guidelines about all that:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

In my experience, it is much easier to sell a feature to the community
if a patch is organized into independent useful pieces with
refactoring pieces presented on top of the actual feature.  In order
to achieve that `git format-patch` is essential because it is possible
to present a patch set organizing your ideas so as others need to
spend less time trying to figure out what a patch set is doing when
doing a review.  format-patch with `git am` is also quite good to
track the addition of new files or the removal of old files.  Writing
your ideas in the commit logs can also bring a lot of insight for
anybody reading your patches.

For simpler and localized changes, using something like git diff would
be also OK that can be applied with a simple `patch` command can also
be fine.  I've done plenty of work with patches sent to the lists this
way for bug fixes.  Of course this is case-by-case, for rather complex
bug fixes format-patch can still be a huge gain of time when reading
somebody else's ideas on a specific matter.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: How to send patch with so many files changes?
@ 2024-09-25 01:20  Tony Wayne <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Tony Wayne @ 2024-09-25 01:20 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; pgsql-hackers

On Wed, Sep 25, 2024 at 6:36 AM Michael Paquier <[email protected]> wrote:

> On Wed, Sep 25, 2024 at 06:19:39AM +0530, Tony Wayne wrote:
> > These changes are for core ,I think it would be better to either move
> whole
> > changes to core or contrib as an extension.
>
> Please avoid top-posting.  The community mailing lists use
> bottom-posting, to ease discussions.  See:
> https://en.wikipedia.org/wiki/Posting_style#Bottom-posting
>
> Thanks for the feedback.

> > On Wed, Sep 25, 2024 at 6:09 AM David G. Johnston <
> > [email protected]> wrote:
> >> Usually you'd want to use format-patch so your commit message(s) make it
> >> into the artifact.  Especially for something complex/large.
>
> The community wiki has some guidelines about all that:
> https://wiki.postgresql.org/wiki/Submitting_a_Patch
>
> In my experience, it is much easier to sell a feature to the community
> if a patch is organized into independent useful pieces with
> refactoring pieces presented on top of the actual feature.  In order
> to achieve that `git format-patch` is essential because it is possible
> to present a patch set organizing your ideas so as others need to
> spend less time trying to figure out what a patch set is doing when
> doing a review.  format-patch with `git am` is also quite good to
> track the addition of new files or the removal of old files.  Writing
> your ideas in the commit logs can also bring a lot of insight for
> anybody reading your patches.
>
> For simpler and localized changes, using something like git diff would
> be also OK that can be applied with a simple `patch` command can also
> be fine.  I've done plenty of work with patches sent to the lists this
> way for bug fixes.  Of course this is case-by-case, for rather complex
> bug fixes format-patch can still be a huge gain of time when reading
> somebody else's ideas on a specific matter.
> --
> Michael
>
Thanks, I got it 👍.


^ permalink  raw  reply  [nested|flat] 9+ messages in thread

end of thread, other threads:[~2024-09-25 01:20 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-08-18 09:28 [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. Michael Banck <[email protected]>
2017-09-10 16:07 [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. Michael Banck <[email protected]>
2017-09-10 16:07 [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. Michael Banck <[email protected]>
2020-11-18 07:54 [PATCH v6 1/3] CatCache expiration feature Kyotaro Horiguchi <[email protected]>
2024-09-25 00:36 Re: How to send patch with so many files changes? Tony Wayne <[email protected]>
2024-09-25 00:38 ` Re: How to send patch with so many files changes? David G. Johnston <[email protected]>
2024-09-25 00:49   ` Re: How to send patch with so many files changes? Tony Wayne <[email protected]>
2024-09-25 01:06     ` Re: How to send patch with so many files changes? Michael Paquier <[email protected]>
2024-09-25 01:20       ` Re: How to send patch with so many files changes? Tony Wayne <[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