Restart scheduler on error

If the scheduler receives an error, it will never restart again since
`bgw_restart_time` is set to `BGW_NEVER_RESTART`, which will prevent
all jobs from executing.

This commit adds the GUC `timescaledb.bgw_scheduler_restart_time` that
can be set to the restart time for the scheduler. It defaults
to 60 seconds, which is the default restart interval for background
workers PostgreSQL defines.

It also adds `timescaledb.debug_bgw_scheduler_exit_status` to be able
to shutdown the scheduler with a non-zero exit status, which allows the
restart functionality to be tested.

Fixes timescale#5091

.unreleased/feature_6195

@@ -0,0 +1 @@
+Implements: #6195 Restart scheduler on error

src/bgw/scheduler.c

@@ -879,6 +879,7 @@ ts_bgw_scheduler_process(int32 run_for_interval_ms,
 	wait_for_all_jobs_to_shutdown();
 	check_for_stopped_and_timed_out_jobs();
 	scheduled_jobs = NIL;
+	proc_exit(ts_debug_bgw_scheduler_exit_status);
 }
 
 static void

src/guc.c

@@ -183,6 +183,20 @@ bool ts_guc_debug_require_batch_sorted_merge = false;
 
 bool ts_guc_debug_allow_cagg_with_deprecated_funcs = false;
 
+/*
+ * Exit code for the scheduler.
+ *
+ * Normally it exits with a zero which means that it will not restart. If an
+ * error is raised, it exits with error code 1, which will trigger a
+ * restart.
+ *
+ * This variable exists to be able to trigger a restart for a normal exit,
+ * which is useful when debugging.
+ *
+ * See backend/postmaster/bgworker.c
+ */
+int ts_debug_bgw_scheduler_exit_status = 0;
+
 #ifdef TS_DEBUG
 bool ts_shutdown_bgw = false;
 char *ts_current_timestamp_mock = NULL;
@@ -1067,6 +1081,19 @@ _guc_init(void)
 							 /* assign_hook= */ NULL,
 							 /* show_hook= */ NULL);
 
+	DefineCustomIntVariable(/* name= */ MAKE_EXTOPTION("debug_bgw_scheduler_exit_status"),
+							/* short_desc= */ "exit status to use when shutting down the scheduler",
+							/* long_desc= */ "this is for debugging purposes",
+							/* valueAddr= */ &ts_debug_bgw_scheduler_exit_status,
+							/* bootValue= */ 0,
+							/* minValue= */ 0,
+							/* maxValue= */ 255,
+							/* context= */ PGC_SIGHUP,
+							/* flags= */ 0,
+							/* check_hook= */ NULL,
+							/* assign_hook= */ NULL,
+							/* show_hook= */ NULL);
+
 	DefineCustomStringVariable(/* name= */ MAKE_EXTOPTION("current_timestamp_mock"),
 							   /* short_desc= */ "set the current timestamp",
 							   /* long_desc= */ "this is for debugging purposes",

src/guc.h

@@ -92,6 +92,14 @@ extern TSDLLEXPORT bool ts_guc_auto_sparse_indexes;
 extern TSDLLEXPORT bool ts_guc_enable_columnarscan;
 extern TSDLLEXPORT int ts_guc_bgw_log_level;
 
+/*
+ * Exit code to use when scheduler exits.
+ *
+ * Mostly used for debugging, but defined also for non-debug builds since that
+ * simplifies the code (and also simplifies debugging non-debug builds).
+ */
+extern TSDLLEXPORT int ts_debug_bgw_scheduler_exit_status;
+
 #ifdef TS_DEBUG
 extern bool ts_shutdown_bgw;
 extern char *ts_current_timestamp_mock;

src/loader/bgw_launcher.c

@@ -23,6 +23,7 @@
 #include <access/htup_details.h>
 #include <access/xact.h>
 #include <catalog/pg_database.h>
+#include <utils/guc.h>
 #include <utils/snapmgr.h>
 
 /* and checking db list for whether we're in a template*/
@@ -40,11 +41,15 @@
 /* for allocating the htab storage */
 #include <utils/memutils.h>
 
+#include <lib/ilist.h>
+#include <postmaster/bgworker_internals.h>
+
 /* for getting settings correct before loading the versioned scheduler */
 #include "catalog/pg_db_role_setting.h"
 
 #include "../compat/compat.h"
 #include "../extension_constants.h"
+#include "../utils.h"
 #include "bgw_counter.h"
 #include "bgw_launcher.h"
 #include "bgw_message_queue.h"
@@ -84,6 +89,8 @@ typedef enum SchedulerState
 
 static volatile sig_atomic_t got_SIGHUP = false;
 
+int ts_guc_bgw_scheduler_restart_time_sec = BGW_DEFAULT_RESTART_INTERVAL;
+
 static void
 launcher_sighup(SIGNAL_ARGS)
 {
@@ -124,6 +131,7 @@ typedef struct DbHashEntry
 } DbHashEntry;
 
 static void scheduler_state_trans_enabled_to_allocated(DbHashEntry *entry);
+static void scheduler_modify_state(DbHashEntry *entry, SchedulerState new_state);
 
 static void
 bgw_on_postmaster_death(void)
@@ -238,13 +246,27 @@ terminate_background_worker(BackgroundWorkerHandle *handle)
 }
 
 extern void
-ts_bgw_cluster_launcher_register(void)
+ts_bgw_cluster_launcher_init(void)
 {
 	BackgroundWorker worker;
 
+	DefineCustomIntVariable(/* name= */ MAKE_EXTOPTION("bgw_scheduler_restart_time"),
+							/* short_desc= */ "Restart time for scheduler in seconds",
+							/* long_desc= */
+							"The number of seconds until the scheduler restart on failure.",
+							/* valueAddr= */ &ts_guc_bgw_scheduler_restart_time_sec,
+							/* bootValue= */ BGW_DEFAULT_RESTART_INTERVAL,
+							/* minValue= */ 1,
+							/* maxValue= */ 3600,
+							/* context= */ PGC_SIGHUP,
+							/* flags= */ GUC_UNIT_S,
+							/* check_hook= */ NULL,
+							/* assign_hook= */ NULL,
+							/* show_hook= */ NULL);
+
 	memset(&worker, 0, sizeof(worker));
 	/* set up worker settings for our main worker */
-	snprintf(worker.bgw_name, BGW_MAXLEN, "TimescaleDB Background Worker Launcher");
+	snprintf(worker.bgw_name, BGW_MAXLEN, TS_BGW_TYPE_LAUNCHER);
 	worker.bgw_flags = BGWORKER_SHMEM_ACCESS | BGWORKER_BACKEND_DATABASE_CONNECTION;
 	worker.bgw_restart_time = BGW_LAUNCHER_RESTART_TIME_S;
 
@@ -274,9 +296,10 @@ register_entrypoint_for_db(Oid db_id, VirtualTransactionId vxid, BackgroundWorke
 	BackgroundWorker worker;
 
 	memset(&worker, 0, sizeof(worker));
-	snprintf(worker.bgw_name, BGW_MAXLEN, "TimescaleDB Background Worker Scheduler");
+	snprintf(worker.bgw_type, BGW_MAXLEN, TS_BGW_TYPE_SCHEDULER);
+	snprintf(worker.bgw_name, BGW_MAXLEN, "%s for database %d", TS_BGW_TYPE_SCHEDULER, db_id);
 	worker.bgw_flags = BGWORKER_SHMEM_ACCESS | BGWORKER_BACKEND_DATABASE_CONNECTION;
-	worker.bgw_restart_time = BGW_NEVER_RESTART;
+	worker.bgw_restart_time = ts_guc_bgw_scheduler_restart_time_sec,
 	worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
 	snprintf(worker.bgw_library_name, BGW_MAXLEN, EXTENSION_NAME);
 	snprintf(worker.bgw_function_name, BGW_MAXLEN, BGW_ENTRYPOINT_FUNCNAME);
@@ -304,6 +327,20 @@ init_database_htab(void)
 					   HASH_BLOBS | HASH_CONTEXT | HASH_ELEM);
 }
 
+static DbHashEntry *
+db_hash_entry_update(HTAB *db_htab, Oid db_oid, SchedulerState state)
+{
+	bool found;
+	DbHashEntry *entry = (DbHashEntry *) hash_search(db_htab, &db_oid, HASH_FIND, &found);
+	if (!found)
+	{
+		elog(LOG, "could not find database entry for %d", db_oid);
+		return NULL;
+	}
+	scheduler_modify_state(entry, state);
+	return entry;
+}
+
 /* Insert a scheduler entry into the hash table. Correctly set entry values. */
 static DbHashEntry *
 db_hash_entry_create_if_not_exists(HTAB *db_htab, Oid db_oid)
@@ -332,15 +369,43 @@ db_hash_entry_create_if_not_exists(HTAB *db_htab, Oid db_oid)
 	return db_he;
 }
 
+/*
+ * Update the database hash table with information from the registered
+ * background worker list.
+ *
+ * This list contains background workers that are currently assigned a slot
+ * and are either running or will soon be running.
+ *
+ * In either case, we should not start new schedulers if there are old ones
+ * available from a previous launcher and they have not terminated, so we set
+ * the state to "STARTED" since it already has a slot.
+ *
+ * We do not update the state if it is terminated because the postmaster will
+ * reclaim this slot later, so we instead create a new scheduler.
+ */
+static void
+update_database_htab(HTAB *db_htab)
+{
+	slist_iter siter;
+	slist_foreach(siter, &BackgroundWorkerList)
+	{
+		RegisteredBgWorker *rw = slist_container(RegisteredBgWorker, rw_lnode, siter.cur);
+
+		/* We store the database id in the main arg for schedulers, so we can
+		 * fetch the database OID from there. */
+		if (strcmp(rw->rw_worker.bgw_type, TS_BGW_TYPE_SCHEDULER) == 0 && !rw->rw_terminate)
+			db_hash_entry_update(db_htab, rw->rw_worker.bgw_main_arg, STARTED);
+	}
+}
+
 /*
  * Model this on autovacuum.c -> get_database_list.
  *
- * Note that we are not doing
- * all the things around memory context that they do, because the hashtable
- * we're using to store db entries is automatically created in its own memory
- * context (a child of TopMemoryContext) This can get called at two different
- * times 1) when the cluster launcher starts and is looking for dbs and 2) if
- * it restarts due to a postmaster signal.
+ * Note that we are not doing all the things around memory context that they
+ * do, because the hashtable we're using to store db entries is automatically
+ * created in its own memory context (a child of TopMemoryContext) This can
+ * get called at two different times 1) when the cluster launcher starts and
+ * is looking for dbs and 2) if it restarts due to a postmaster signal.
  */
 static void
 populate_database_htab(HTAB *db_htab)
@@ -758,6 +823,7 @@ ts_bgw_cluster_launcher_main(PG_FUNCTION_ARGS)
 	*htab_storage = db_htab;
 
 	populate_database_htab(db_htab);
+	update_database_htab(db_htab);
 
 	while (true)
 	{

src/loader/bgw_launcher.h

@@ -8,7 +8,12 @@
 #include <postgres.h>
 #include <fmgr.h>
 
-extern void ts_bgw_cluster_launcher_register(void);
+#define TS_BGW_TYPE_LAUNCHER "TimescaleDB Background Worker Launcher"
+#define TS_BGW_TYPE_SCHEDULER "TimescaleDB Background Worker Scheduler"
+
+extern int ts_guc_bgw_scheduler_restart_time_sec;
+
+extern void ts_bgw_cluster_launcher_init(void);
 
 /*called by postmaster at launcher bgw startup*/
 TSDLLEXPORT extern Datum ts_bgw_cluster_launcher_main(PG_FUNCTION_ARGS);

src/loader/loader.c

@@ -591,7 +591,7 @@ _PG_init(void)
 	timescaledb_shmem_request_hook();
 #endif
 
-	ts_bgw_cluster_launcher_register();
+	ts_bgw_cluster_launcher_init();
 	ts_bgw_counter_setup_gucs();
 	ts_bgw_interface_register_api_version();
 

tsl/test/expected/bgw_scheduler_restart.out

@@ -0,0 +1,85 @@
+-- This file and its contents are licensed under the Timescale License.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-TIMESCALE for a copy of the license.
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE VIEW tsdb_bgw AS
+       SELECT datname, pid, backend_type, application_name
+         FROM pg_stat_activity
+        WHERE application_name LIKE '%TimescaleDB%'
+     ORDER BY datname, backend_type, application_name;
+-- Show the default scheduler restart time
+SHOW timescaledb.bgw_scheduler_restart_time;
+ timescaledb.bgw_scheduler_restart_time 
+----------------------------------------
+ 1min
+(1 row)
+
+SELECT _timescaledb_functions.start_background_workers();
+ start_background_workers 
+--------------------------
+ t
+(1 row)
+
+SELECT pg_sleep(10);		-- Wait for scheduler to start.
+ pg_sleep 
+----------
+
+(1 row)
+
+SELECT datname, application_name FROM tsdb_bgw;
+         datname          |            application_name             
+--------------------------+-----------------------------------------
+ db_bgw_scheduler_restart | TimescaleDB Background Worker Scheduler
+                          | TimescaleDB Background Worker Launcher
+(2 rows)
+
+ALTER SYSTEM SET timescaledb.shutdown_bgw_scheduler TO 'on';
+ALTER SYSTEM SET timescaledb.debug_bgw_scheduler_exit_status TO 1;
+SELECT pg_reload_conf();
+ pg_reload_conf 
+----------------
+ t
+(1 row)
+
+SELECT pg_sleep(20);		-- Wait for scheduler to exit.
+ pg_sleep 
+----------
+
+(1 row)
+
+SELECT datname, application_name FROM tsdb_bgw;
+ datname |            application_name            
+---------+----------------------------------------
+         | TimescaleDB Background Worker Launcher
+(1 row)
+
+ALTER SYSTEM RESET timescaledb.shutdown_bgw_scheduler;
+ALTER SYSTEM RESET timescaledb.debug_bgw_scheduler_exit_status;
+SELECT pg_reload_conf();
+ pg_reload_conf 
+----------------
+ t
+(1 row)
+
+SELECT pg_sleep(60);		-- Wait for scheduler to restart.
+ pg_sleep 
+----------
+
+(1 row)
+
+SELECT datname, application_name FROM tsdb_bgw;
+         datname          |            application_name             
+--------------------------+-----------------------------------------
+ db_bgw_scheduler_restart | TimescaleDB Background Worker Scheduler
+                          | TimescaleDB Background Worker Launcher
+(2 rows)
+
+SELECT pg_terminate_backend(pid)
+  FROM pg_stat_activity
+ WHERE datname = :'TEST_DBNAME'
+   AND application_name LIKE 'TimescaleDB%';
+ pg_terminate_backend 
+----------------------
+ t
+(1 row)
+