Avoid unnecessary scheduler restarts

When a user creates a database based on a template a FATAL error will
be generated, which causes the scheduler to restart. The same happens
if a FATAL error is generate in any other way, e.g., if the database
does not exist.

If you use an actual template database (like template0) this can
cause endless errors and retries wasting bgw slots. Hence, we avoid
starting the bgw scheduler for databases that does not exist and
databases we cannot connect to.

In addition to when creating a new database, a re-starting scheduler
might generate errors if the database has gone away or if permissions
for the database changed. This will trigger an endlessly restarting
scheduler.

We fix this issue by overriding connection block when connecting to the
database and perform the check ourselves to avoid a fatal error, and
capturing any non-fatal error that can be generated in the scheduler
startup phase and instead report the error ourselves and exiting with
exit code 0 (which means the scheduler will not restart).

If an error occurs inside the main scheduler process, this will still
trigger a restart.

Co-authored-by: Fabrízio de Royes Mello <[email protected]>
Co-authored-by: Mats Kindahl <[email protected]>

Author: 3 people

.unreleased/pr_7834

@@ -0,0 +1 @@
+Fixes: #7834 Avoid unnecessary scheduler restarts

src/loader/bgw_launcher.c

@@ -899,7 +899,7 @@ ts_bgw_cluster_launcher_main(PG_FUNCTION_ARGS)
  * Modelled on autovacuum.c -> do_autovacuum.
  */
 static void
-database_is_template_check(void)
+database_checks(void)
 {
 	Form_pg_database pgdb;
 	HeapTuple tuple;
@@ -911,9 +911,17 @@ database_is_template_check(void)
 						"syscache")));
 
 	pgdb = (Form_pg_database) GETSTRUCT(tuple);
+
+	if (!pgdb->datallowconn)
+		ereport(ERROR,
+				(errmsg("background worker \"%s\" trying to connect to database that does not "
+						"allow connections, exiting",
+						MyBgworkerEntry->bgw_name)));
+
 	if (pgdb->datistemplate)
 		ereport(ERROR,
-				(errmsg("TimescaleDB background worker connected to template database, exiting")));
+				(errmsg("background worker \"%s\" trying to connect to template database, exiting",
+						MyBgworkerEntry->bgw_name)));
 
 	ReleaseSysCache(tuple);
 }
@@ -951,6 +959,63 @@ process_settings(Oid databaseid)
 	table_close(relsetting, AccessShareLock);
 }
 
+/*
+ * Get the versioned scheduler for the database.
+ *
+ * This captures any errors generated while fetching information and print
+ * them out, but does not propagate the error further since that might trigger
+ * a restart.
+ */
+static PGFunction
+get_versioned_scheduler()
+{
+	PG_TRY();
+	{
+		bool ts_installed = false;
+		char version[MAX_VERSION_LEN];
+
+		/*
+		 * now we can start our transaction and get the version currently
+		 * installed
+		 */
+		StartTransactionCommand();
+		(void) GetTransactionSnapshot();
+
+		/*
+		 * Check whether a database is a template database and raise an error if
+		 * so, as we don't want to run in template dbs.
+		 */
+		database_checks();
+		/*  Process any config changes caused by an ALTER DATABASE */
+		process_settings(MyDatabaseId);
+		ts_installed = ts_loader_extension_exists();
+		if (ts_installed)
+			strlcpy(version, ts_loader_extension_version(), MAX_VERSION_LEN);
+
+		ts_loader_extension_check();
+		CommitTransactionCommand();
+		if (ts_installed)
+		{
+			char soname[MAX_SO_NAME_LEN];
+			snprintf(soname, MAX_SO_NAME_LEN, "%s-%s", EXTENSION_SO, version);
+			PGFunction versioned_scheduler_main =
+				load_external_function(soname, BGW_DB_SCHEDULER_FUNCNAME, false, NULL);
+			if (versioned_scheduler_main == NULL)
+				ereport(ERROR,
+						(errmsg("TimescaleDB version %s does not have a background worker, exiting",
+								soname)));
+			return versioned_scheduler_main;
+		}
+	}
+	PG_CATCH();
+	{
+		EmitErrorReport();
+		FlushErrorState();
+	}
+	PG_END_TRY();
+	return NULL;
+}
+
 /*
  * This can be run either from the cluster launcher at db_startup time, or
  * in the case of an install/uninstall/update of the extension, in the
@@ -964,14 +1029,18 @@ extern Datum
 ts_bgw_db_scheduler_entrypoint(PG_FUNCTION_ARGS)
 {
 	Oid db_id = DatumGetObjectId(MyBgworkerEntry->bgw_main_arg);
-	bool ts_installed = false;
-	char version[MAX_VERSION_LEN];
 	VirtualTransactionId vxid;
 
 	pqsignal(SIGINT, StatementCancelHandler);
 	pqsignal(SIGTERM, die);
 	BackgroundWorkerUnblockSignals();
-	BackgroundWorkerInitializeConnectionByOid(db_id, InvalidOid, 0);
+
+	/*
+	 * Connecting to a database that does not allow connections will generate
+	 * a FATAL error, which might trigger restarts, so we override this check
+	 * and do it ourselves.
+	 */
+	BackgroundWorkerInitializeConnectionByOid(db_id, InvalidOid, BGWORKER_BYPASS_ALLOWCONN);
 	pgstat_report_appname(MyBgworkerEntry->bgw_name);
 
 	/*
@@ -988,40 +1057,13 @@ ts_bgw_db_scheduler_entrypoint(PG_FUNCTION_ARGS)
 	CommitTransactionCommand();
 
 	/*
-	 * now we can start our transaction and get the version currently
-	 * installed
-	 */
-	StartTransactionCommand();
-	(void) GetTransactionSnapshot();
-
-	/*
-	 * Check whether a database is a template database and raise an error if
-	 * so, as we don't want to run in template dbs.
+	 * Essentially we morph into the versioned worker here, if there is one.
+	 *
+	 * If an error is generated here, we should trigger a restart (if the
+	 * scheduler is configured for that).
 	 */
-	database_is_template_check();
-	/*  Process any config changes caused by an ALTER DATABASE */
-	process_settings(MyDatabaseId);
-	ts_installed = ts_loader_extension_exists();
-	if (ts_installed)
-		strlcpy(version, ts_loader_extension_version(), MAX_VERSION_LEN);
-
-	ts_loader_extension_check();
-	CommitTransactionCommand();
-	if (ts_installed)
-	{
-		char soname[MAX_SO_NAME_LEN];
-		PGFunction versioned_scheduler_main;
-
-		snprintf(soname, MAX_SO_NAME_LEN, "%s-%s", EXTENSION_SO, version);
-		versioned_scheduler_main =
-			load_external_function(soname, BGW_DB_SCHEDULER_FUNCNAME, false, NULL);
-		if (versioned_scheduler_main == NULL)
-			ereport(LOG,
-					(errmsg("TimescaleDB version %s does not have a background worker, exiting",
-							soname)));
-		else /* essentially we morph into the versioned
-			  * worker here */
-			DirectFunctionCall1(versioned_scheduler_main, ObjectIdGetDatum(InvalidOid));
-	}
+	PGFunction versioned_scheduler_main = get_versioned_scheduler();
+	if (versioned_scheduler_main)
+		DirectFunctionCall1(versioned_scheduler_main, ObjectIdGetDatum(InvalidOid));
 	PG_RETURN_VOID();
 }

src/loader/loader.c

@@ -8,6 +8,7 @@
 #include <access/heapam.h>
 #include <access/parallel.h>
 #include <access/xact.h>
+#include <catalog/pg_database.h>
 #include <commands/dbcommands.h>
 #include <commands/defrem.h>
 #include <commands/user.h>
@@ -390,6 +391,39 @@ stop_workers_on_db_drop(DropdbStmt *drop_db_statement)
 	}
 }
 
+static bool
+database_allowconn(const Oid db_oid)
+{
+	Relation pg_database;
+	ScanKeyData entry[1];
+	SysScanDesc scan;
+	HeapTuple dbtuple;
+	bool allowconn = false;
+
+	pg_database = table_open(DatabaseRelationId, AccessShareLock);
+	ScanKeyInit(&entry[0],
+				Anum_pg_database_oid,
+				BTEqualStrategyNumber,
+				F_OIDEQ,
+				ObjectIdGetDatum(db_oid));
+	scan = systable_beginscan(pg_database, DatabaseOidIndexId, true, NULL, 1, entry);
+
+	dbtuple = systable_getnext(scan);
+
+	/* We assume that there can be at most one matching tuple */
+	if (!HeapTupleIsValid(dbtuple))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_DATABASE),
+				 errmsg("database with OID \"%u\" does not exist", db_oid)));
+
+	allowconn = ((Form_pg_database) GETSTRUCT(dbtuple))->datallowconn;
+
+	systable_endscan(scan);
+	table_close(pg_database, AccessShareLock);
+
+	return allowconn;
+}
+
 static void
 post_analyze_hook(ParseState *pstate, Query *query, JumbleState *jstate)
 {
@@ -440,7 +474,7 @@ post_analyze_hook(ParseState *pstate, Query *query, JumbleState *jstate)
 					{
 						Oid db_oid = get_database_oid(defGetString(option), false);
 
-						if (OidIsValid(db_oid))
+						if (OidIsValid(db_oid) && database_allowconn(db_oid))
 							ts_bgw_message_send_and_wait(RESTART, db_oid);
 					}
 				}