public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v5 2/2] Add a --outdated option to reindexdb
3+ messages / 3 participants
[nested] [flat]

* [PATCH v5 2/2] Add a --outdated option to reindexdb
@ 2021-02-24 17:33  Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Julien Rouhaud @ 2021-02-24 17:33 UTC (permalink / raw)

This uses the new OUTDATED option for REINDEX.  If user asks for multiple job,
the list of tables to process will be sorted by the total size of underlying
indexes that have outdated dependency.

Author: Julien Rouhaud <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/20201203093143.GA64934%40nol
---
 src/bin/scripts/reindexdb.c        | 143 ++++++++++++++++++++++++-----
 src/bin/scripts/t/090_reindexdb.pl |  34 ++++++-
 2 files changed, 151 insertions(+), 26 deletions(-)

diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index cf28176243..369d164e65 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -35,20 +35,23 @@ typedef enum ReindexType
 static SimpleStringList *get_parallel_object_list(PGconn *conn,
 												  ReindexType type,
 												  SimpleStringList *user_list,
+												  bool outdated,
 												  bool echo);
 static void reindex_one_database(const ConnParams *cparams, ReindexType type,
 								 SimpleStringList *user_list,
 								 const char *progname,
 								 bool echo, bool verbose, bool concurrently,
-								 int concurrentCons, const char *tablespace);
+								 int concurrentCons, const char *tablespace,
+								 bool outdated);
 static void reindex_all_databases(ConnParams *cparams,
 								  const char *progname, bool echo,
 								  bool quiet, bool verbose, bool concurrently,
-								  int concurrentCons, const char *tablespace);
+								  int concurrentCons, const char *tablespace,
+								  bool outdated);
 static void run_reindex_command(PGconn *conn, ReindexType type,
 								const char *name, bool echo, bool verbose,
 								bool concurrently, bool async,
-								const char *tablespace);
+								const char *tablespace, bool outdated);
 
 static void help(const char *progname);
 
@@ -74,6 +77,7 @@ main(int argc, char *argv[])
 		{"concurrently", no_argument, NULL, 1},
 		{"maintenance-db", required_argument, NULL, 2},
 		{"tablespace", required_argument, NULL, 3},
+		{"outdated", no_argument, NULL, 4},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -95,6 +99,7 @@ main(int argc, char *argv[])
 	bool		quiet = false;
 	bool		verbose = false;
 	bool		concurrently = false;
+	bool		outdated = false;
 	SimpleStringList indexes = {NULL, NULL};
 	SimpleStringList tables = {NULL, NULL};
 	SimpleStringList schemas = {NULL, NULL};
@@ -170,6 +175,9 @@ main(int argc, char *argv[])
 			case 3:
 				tablespace = pg_strdup(optarg);
 				break;
+			case 4:
+				outdated = true;
+				break;
 			default:
 				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 				exit(1);
@@ -234,7 +242,8 @@ main(int argc, char *argv[])
 		cparams.dbname = maintenance_db;
 
 		reindex_all_databases(&cparams, progname, echo, quiet, verbose,
-							  concurrently, concurrentCons, tablespace);
+							  concurrently, concurrentCons, tablespace,
+							  outdated);
 	}
 	else if (syscatalog)
 	{
@@ -253,12 +262,17 @@ main(int argc, char *argv[])
 			pg_log_error("cannot reindex specific index(es) and system catalogs at the same time");
 			exit(1);
 		}
-
 		if (concurrentCons > 1)
 		{
 			pg_log_error("cannot use multiple jobs to reindex system catalogs");
 			exit(1);
 		}
+		if (outdated)
+		{
+			pg_log_error("cannot filter indexes having outdated dependencies "
+						 "and reindex system catalogs at the same time");
+			exit(1);
+		}
 
 		if (dbname == NULL)
 		{
@@ -274,7 +288,7 @@ main(int argc, char *argv[])
 
 		reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
 							 progname, echo, verbose,
-							 concurrently, 1, tablespace);
+							 concurrently, 1, tablespace, outdated);
 	}
 	else
 	{
@@ -304,17 +318,20 @@ main(int argc, char *argv[])
 		if (schemas.head != NULL)
 			reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas,
 								 progname, echo, verbose,
-								 concurrently, concurrentCons, tablespace);
+								 concurrently, concurrentCons, tablespace,
+								 outdated);
 
 		if (indexes.head != NULL)
 			reindex_one_database(&cparams, REINDEX_INDEX, &indexes,
 								 progname, echo, verbose,
-								 concurrently, 1, tablespace);
+								 concurrently, 1, tablespace,
+								 outdated);
 
 		if (tables.head != NULL)
 			reindex_one_database(&cparams, REINDEX_TABLE, &tables,
 								 progname, echo, verbose,
-								 concurrently, concurrentCons, tablespace);
+								 concurrently, concurrentCons, tablespace,
+								 outdated);
 
 		/*
 		 * reindex database only if neither index nor table nor schema is
@@ -323,7 +340,8 @@ main(int argc, char *argv[])
 		if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
 			reindex_one_database(&cparams, REINDEX_DATABASE, NULL,
 								 progname, echo, verbose,
-								 concurrently, concurrentCons, tablespace);
+								 concurrently, concurrentCons, tablespace,
+								 outdated);
 	}
 
 	exit(0);
@@ -334,7 +352,7 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
 					 SimpleStringList *user_list,
 					 const char *progname, bool echo,
 					 bool verbose, bool concurrently, int concurrentCons,
-					 const char *tablespace)
+					 const char *tablespace, bool outdated)
 {
 	PGconn	   *conn;
 	SimpleStringListCell *cell;
@@ -363,6 +381,14 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
 		exit(1);
 	}
 
+	if (outdated && PQserverVersion(conn) < 140000)
+	{
+		PQfinish(conn);
+		pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s",
+					 "outdated", "14");
+		exit(1);
+	}
+
 	if (!parallel)
 	{
 		switch (process_type)
@@ -399,14 +425,24 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
 				 */
 				if (concurrently)
 					pg_log_warning("cannot reindex system catalogs concurrently, skipping all");
+				else if (outdated)
+				{
+					/*
+					 * The only supported kind of object that can be outdated
+					 * is collation.  No system catalog has any index that can
+					 * depend on an outdated collation, so skip system
+					 * catalogs.
+					 */
+				}
 				else
 					run_reindex_command(conn, REINDEX_SYSTEM, PQdb(conn), echo,
 										verbose, concurrently, false,
-										tablespace);
+										tablespace, outdated);
 
 				/* Build a list of relations from the database */
 				process_list = get_parallel_object_list(conn, process_type,
-														user_list, echo);
+														user_list, outdated,
+														echo);
 				process_type = REINDEX_TABLE;
 
 				/* Bail out if nothing to process */
@@ -419,7 +455,8 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
 
 				/* Build a list of relations from all the schemas */
 				process_list = get_parallel_object_list(conn, process_type,
-														user_list, echo);
+														user_list, outdated,
+														echo);
 				process_type = REINDEX_TABLE;
 
 				/* Bail out if nothing to process */
@@ -484,7 +521,8 @@ reindex_one_database(const ConnParams *cparams, ReindexType type,
 
 		ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL);
 		run_reindex_command(free_slot->connection, process_type, objname,
-							echo, verbose, concurrently, true, tablespace);
+							echo, verbose, concurrently, true, tablespace,
+							outdated);
 
 		cell = cell->next;
 	} while (cell != NULL);
@@ -509,7 +547,7 @@ finish:
 static void
 run_reindex_command(PGconn *conn, ReindexType type, const char *name,
 					bool echo, bool verbose, bool concurrently, bool async,
-					const char *tablespace)
+					const char *tablespace, bool outdated)
 {
 	const char *paren = "(";
 	const char *comma = ", ";
@@ -536,6 +574,12 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
 		sep = comma;
 	}
 
+	if (outdated)
+	{
+		appendPQExpBuffer(&sql, "%sOUTDATED", sep);
+		sep = comma;
+	}
+
 	if (sep != paren)
 		appendPQExpBufferStr(&sql, ") ");
 
@@ -641,7 +685,7 @@ run_reindex_command(PGconn *conn, ReindexType type, const char *name,
  */
 static SimpleStringList *
 get_parallel_object_list(PGconn *conn, ReindexType type,
-						 SimpleStringList *user_list, bool echo)
+						 SimpleStringList *user_list, bool outdated, bool echo)
 {
 	PQExpBufferData catalog_query;
 	PQExpBufferData buf;
@@ -660,16 +704,41 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
 	{
 		case REINDEX_DATABASE:
 			Assert(user_list == NULL);
+
 			appendPQExpBufferStr(&catalog_query,
 								 "SELECT c.relname, ns.nspname\n"
 								 " FROM pg_catalog.pg_class c\n"
 								 " JOIN pg_catalog.pg_namespace ns"
-								 " ON c.relnamespace = ns.oid\n"
+								 " ON c.relnamespace = ns.oid\n");
+
+			if (outdated)
+			{
+				appendPQExpBufferStr(&catalog_query,
+									 " JOIN pg_catalog.pg_index i"
+									 " ON c.oid = i.indrelid\n"
+									 " JOIN pg_catalog.pg_class ci"
+									 " ON i.indexrelid = ci.oid\n");
+			}
+
+			appendPQExpBufferStr(&catalog_query,
 								 " WHERE ns.nspname != 'pg_catalog'\n"
 								 "   AND c.relkind IN ("
 								 CppAsString2(RELKIND_RELATION) ", "
-								 CppAsString2(RELKIND_MATVIEW) ")\n"
-								 " ORDER BY c.relpages DESC;");
+								 CppAsString2(RELKIND_MATVIEW) ")\n");
+
+			if (outdated)
+			{
+				appendPQExpBufferStr(&catalog_query,
+									 " GROUP BY c.relname, ns.nspname\n"
+									 " ORDER BY sum(ci.relpages)"
+									 " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+			}
+			else
+			{
+				appendPQExpBufferStr(&catalog_query,
+									 " ORDER BY c.relpages DESC;");
+			}
+
 			break;
 
 		case REINDEX_SCHEMA:
@@ -687,7 +756,18 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
 									 "SELECT c.relname, ns.nspname\n"
 									 " FROM pg_catalog.pg_class c\n"
 									 " JOIN pg_catalog.pg_namespace ns"
-									 " ON c.relnamespace = ns.oid\n"
+									 " ON c.relnamespace = ns.oid\n");
+
+				if (outdated)
+				{
+					appendPQExpBufferStr(&catalog_query,
+										 " JOIN pg_catalog.pg_index i"
+										 " ON c.oid = i.indrelid\n"
+										 " JOIN pg_catalog.pg_class ci"
+										 " ON i.indexrelid = ci.oid\n");
+				}
+
+				appendPQExpBufferStr(&catalog_query,
 									 " WHERE c.relkind IN ("
 									 CppAsString2(RELKIND_RELATION) ", "
 									 CppAsString2(RELKIND_MATVIEW) ")\n"
@@ -705,8 +785,20 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
 					appendStringLiteralConn(&catalog_query, nspname, conn);
 				}
 
-				appendPQExpBufferStr(&catalog_query, ")\n"
-									 " ORDER BY c.relpages DESC;");
+				appendPQExpBufferStr(&catalog_query, ")\n");
+
+				if (outdated)
+				{
+					appendPQExpBufferStr(&catalog_query,
+										 " GROUP BY c.relname, ns.nspname\n"
+										 " ORDER BY sum(ci.relpages)"
+										 " FILTER (WHERE pg_catalog.pg_index_has_outdated_dependency(ci.oid)) DESC;");
+				}
+				else
+				{
+					appendPQExpBufferStr(&catalog_query,
+										 " ORDER BY c.relpages DESC;");
+				}
 			}
 			break;
 
@@ -754,7 +846,7 @@ static void
 reindex_all_databases(ConnParams *cparams,
 					  const char *progname, bool echo, bool quiet, bool verbose,
 					  bool concurrently, int concurrentCons,
-					  const char *tablespace)
+					  const char *tablespace, bool outdated)
 {
 	PGconn	   *conn;
 	PGresult   *result;
@@ -778,7 +870,7 @@ reindex_all_databases(ConnParams *cparams,
 
 		reindex_one_database(cparams, REINDEX_DATABASE, NULL,
 							 progname, echo, verbose, concurrently,
-							 concurrentCons, tablespace);
+							 concurrentCons, tablespace, outdated);
 	}
 
 	PQclear(result);
@@ -797,6 +889,7 @@ help(const char *progname)
 	printf(_("  -e, --echo                   show the commands being sent to the server\n"));
 	printf(_("  -i, --index=INDEX            recreate specific index(es) only\n"));
 	printf(_("  -j, --jobs=NUM               use this many concurrent connections to reindex\n"));
+	printf(_("      --outdated               only process indexes having outdated depencies\n"));
 	printf(_("  -q, --quiet                  don't write any messages\n"));
 	printf(_("  -s, --system                 reindex system catalogs\n"));
 	printf(_("  -S, --schema=SCHEMA          reindex specific schema(s) only\n"));
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 159b637230..b60cac6081 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -3,7 +3,7 @@ use warnings;
 
 use PostgresNode;
 use TestLib;
-use Test::More tests => 58;
+use Test::More tests => 70;
 
 program_help_ok('reindexdb');
 program_version_ok('reindexdb');
@@ -174,6 +174,9 @@ $node->command_fails(
 $node->command_fails(
 	[ 'reindexdb', '-j', '2', '-i', 'i1', 'postgres' ],
 	'parallel reindexdb cannot process indexes');
+$node->command_fails(
+	[ 'reindexdb', '-s', '--outdated' ],
+	'cannot reindex system catalog and filter indexes having outdated dependencies');
 $node->issues_sql_like(
 	[ 'reindexdb', '-j', '2', 'postgres' ],
 	qr/statement:\ REINDEX SYSTEM postgres;
@@ -196,3 +199,32 @@ $node->command_checks_all(
 		qr/^reindexdb: warning: cannot reindex system catalogs concurrently, skipping all/s
 	],
 	'parallel reindexdb for system with --concurrently skips catalogs');
+
+# Temporarily downgrade client-min-message to get the no-op report
+$ENV{PGOPTIONS} = '--client-min-messages=NOTICE';
+$node->command_checks_all(
+	[ 'reindexdb',  '--outdated', '-v', '-t', 's1.t1', 'postgres' ],
+	0,
+	[qr/^$/],
+	[qr/table "t1" has no indexes to reindex/],
+	'verbose reindexdb for outdated dependencies on a specific table reports no-op tables');
+
+$node->command_checks_all(
+	[ 'reindexdb',  '--outdated', '-v', '-d', 'postgres' ],
+	0,
+	[qr/^$/],
+	[qr/^$/],
+	'verbose reindexdb for outdated dependencies database wide silently ignore all tables');
+$node->command_checks_all(
+	[ 'reindexdb',  '--outdated', '-v', '-j', '2', '-d', 'postgres' ],
+	0,
+	[qr/^$/],
+	[qr/table "t1" has no indexes to reindex/],
+	'parallel verbose reindexdb for outdated dependencies database wide reports no-op tables');
+
+# Switch back to WARNING client-min-message
+$ENV{PGOPTIONS} = '--client-min-messages=WARNING';
+$node->issues_sql_like(
+	[ 'reindexdb', '--outdated', '-t', 's1.t1', 'postgres' ],
+	qr/.*statement: REINDEX \(OUTDATED\) TABLE s1\.t1;/,
+	'reindexdb for outdated dependencies specify the OUTDATED keyword');
-- 
2.30.1


--oao5se3im4osqowp--





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

* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-15 21:18  Daniel Gustafsson <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Daniel Gustafsson @ 2025-04-15 21:18 UTC (permalink / raw)
  To: David Rowley <[email protected]>; +Cc: torikoshia <[email protected]>; pgsql-hackers

> On 15 Apr 2025, at 23:03, David Rowley <[email protected]> wrote:

> My vote is to make the levels 1-based in all locations where we output
> the context information.

I agree with this, pg_get_process_memory_contexts() also use 1-based levels
fwiw.

--
Daniel Gustafsson







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

* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-16 14:19  torikoshia <[email protected]>
  parent: Daniel Gustafsson <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: torikoshia @ 2025-04-16 14:19 UTC (permalink / raw)
  To: Daniel Gustafsson <[email protected]>; [email protected]; +Cc: pgsql-hackers

On 2025-04-16 06:18, Daniel Gustafsson wrote:
>> On 15 Apr 2025, at 23:03, David Rowley <[email protected]> wrote:
> 
>> My vote is to make the levels 1-based in all locations where we output
>> the context information.
> 
> I agree with this, pg_get_process_memory_contexts() also use 1-based 
> levels
> fwiw.

+1.
I believe there's no particular issue with starting the level from 1 in 
pg_log_backend_memory_contexts().

Regarding the implementation:
In the initial patch attached, I naïvely incremented the level just 
before emitting the log line.
However, it might be cleaner to simply initialize the level variable to 
1 from the start. This could help avoid unnecessary confusion when 
debugging that part of the code.

Similarly, I noticed that in pg_get_process_memory_contexts(), the level 
is initialized to 0 in ProcessGetMemoryContextInterrupt(void):

     int level = 0;
     ..
     MemoryContextStatsInternal(c, level, 100, 100, &grand_totals, ..

If we want to be consistent, perhaps it would make sense to start from 1 
there as well.

BTW level variable has existed since before pg_backend_memory_contexts 
was introduced — it was originally used for functions that help inspect 
memory contexts via the debugger. Because of that, I think changing this 
would affect not only these functions codes but some older ones.


--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.





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


end of thread, other threads:[~2025-04-16 14:19 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-24 17:33 [PATCH v5 2/2] Add a --outdated option to reindexdb Julien Rouhaud <[email protected]>
2025-04-15 21:18 Re: Align memory context level numbering in pg_log_backend_memory_contexts() Daniel Gustafsson <[email protected]>
2025-04-16 14:19 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() torikoshia <[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