Skip to content

Commit

Permalink
Restart scheduler on error
Browse files Browse the repository at this point in the history
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.

It also ensures that `backend_type` is explicitly set up rather than
copied from `application_name` and add some more information to
`application_name`. It also updates the tests to use `backend_type`
where applicable.
  • Loading branch information
mkindahl committed Dec 12, 2024
1 parent 63f7cc8 commit e8e94bd
Show file tree
Hide file tree
Showing 13 changed files with 281 additions and 47 deletions.
1 change: 1 addition & 0 deletions .unreleased/pr_7527
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7527 Restart scheduler on error
1 change: 1 addition & 0 deletions src/bgw/scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions src/guc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
8 changes: 8 additions & 0 deletions src/guc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
82 changes: 72 additions & 10 deletions src/loader/bgw_launcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -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*/
Expand All @@ -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"
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -304,6 +327,16 @@ 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 = hash_search(db_htab, &db_oid, HASH_FIND, &found);
if (found)
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)
Expand Down Expand Up @@ -332,15 +365,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)
Expand Down Expand Up @@ -758,6 +819,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)
{
Expand Down
7 changes: 6 additions & 1 deletion src/loader/bgw_launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/loader/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
42 changes: 22 additions & 20 deletions test/expected/bgw_launcher.out
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ CREATE DATABASE :TEST_DBNAME_2;
-- Further Note: PG 9.6 changed what appeared in pg_stat_activity, so the launcher doesn't actually show up.
-- we can still test its interactions with its children, but can't test some of the things specific to the launcher.
-- So we've added some bits about the version number as needed.
CREATE VIEW worker_counts as SELECT count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Launcher') as launcher,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME') as single_scheduler,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME_2') as single_2_scheduler,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = 'template1') as template1_scheduler
FROM pg_stat_activity;
CREATE VIEW worker_counts as
SELECT count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Launcher') as launcher,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME') as single_scheduler,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME_2') as single_2_scheduler,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = 'template1') as template1_scheduler
FROM pg_stat_activity;
CREATE FUNCTION wait_worker_counts(launcher_ct INTEGER, scheduler1_ct INTEGER, scheduler2_ct INTEGER, template1_ct INTEGER) RETURNS BOOLEAN LANGUAGE PLPGSQL AS
$BODY$
DECLARE
Expand Down Expand Up @@ -103,7 +104,7 @@ SELECT wait_worker_counts(1,0,1,0);
-- Now let's restart the scheduler in test db 2 and make sure our backend_start changed
SELECT backend_start as orig_backend_start
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2' \gset
-- We'll do this in a txn so that we can see that the worker locks on our txn before continuing
BEGIN;
Expand All @@ -122,7 +123,7 @@ SELECT wait_worker_counts(1,0,1,0);
SELECT (backend_start > :'orig_backend_start'::timestamptz) backend_start_changed,
(wait_event = 'virtualxid') wait_event_changed
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2';
backend_start_changed | wait_event_changed
-----------------------+--------------------
Expand All @@ -138,7 +139,7 @@ SELECT wait_worker_counts(1,0,1,0);

SELECT (wait_event IS DISTINCT FROM 'virtualxid') wait_event_changed
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2';
wait_event_changed
--------------------
Expand Down Expand Up @@ -187,7 +188,7 @@ SELECT wait_worker_counts(1,0,1,0);
-- make sure start is idempotent
SELECT backend_start as orig_backend_start
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2' \gset
-- Since we're doing idempotency tests, we're also going to exercise our queue and start 20 times
SELECT _timescaledb_functions.start_background_workers() as start_background_workers, * FROM generate_series(1,20);
Expand Down Expand Up @@ -227,7 +228,7 @@ FOR i in 1..5
LOOP
SELECT (backend_start = $1::timestamptz) backend_start_unchanged
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = $2 into r;
if(r) THEN
PERFORM pg_sleep(0.1);
Expand Down Expand Up @@ -274,7 +275,7 @@ SELECT wait_worker_counts(1,0,1,0);
-- Now let's restart the scheduler and make sure our backend_start changed
SELECT backend_start as orig_backend_start
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2' \gset
BEGIN;
DROP EXTENSION timescaledb;
Expand All @@ -294,7 +295,7 @@ FOR i in 1..10
LOOP
SELECT (backend_start > $1::timestamptz) backend_start_changed
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = $2 into r;
if(NOT r) THEN
PERFORM pg_sleep(0.1);
Expand All @@ -315,9 +316,9 @@ SELECT wait_greater(:'orig_backend_start',:'TEST_DBNAME_2');
-- Make sure canceling the launcher backend causes a restart of schedulers
SELECT backend_start as orig_backend_start
FROM pg_stat_activity
WHERE application_name = 'TimescaleDB Background Worker Scheduler'
WHERE backend_type = 'TimescaleDB Background Worker Scheduler'
AND datname = :'TEST_DBNAME_2' \gset
SELECT pg_cancel_backend(pid) FROM pg_stat_activity WHERE application_name = 'TimescaleDB Background Worker Launcher';
SELECT pg_cancel_backend(pid) FROM pg_stat_activity WHERE backend_type = 'TimescaleDB Background Worker Launcher';
pg_cancel_backend
-------------------
t
Expand Down Expand Up @@ -445,11 +446,12 @@ ALTER ROLE :ROLE_DEFAULT_PERM_USER WITH NOSUPERUSER;
-- Further Note: PG 9.6 changed what appeared in pg_stat_activity, so the launcher doesn't actually show up.
-- we can still test its interactions with its children, but can't test some of the things specific to the launcher.
-- So we've added some bits about the version number as needed.
CREATE VIEW worker_counts as SELECT count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Launcher') as launcher,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME') as single_scheduler,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME_2') as single_2_scheduler,
count(*) filter (WHERE application_name = 'TimescaleDB Background Worker Scheduler' AND datname = 'template1') as template1_scheduler
FROM pg_stat_activity;
CREATE VIEW worker_counts as
SELECT count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Launcher') as launcher,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME') as single_scheduler,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = :'TEST_DBNAME_2') as single_2_scheduler,
count(*) filter (WHERE backend_type = 'TimescaleDB Background Worker Scheduler' AND datname = 'template1') as template1_scheduler
FROM pg_stat_activity;
CREATE FUNCTION wait_worker_counts(launcher_ct INTEGER, scheduler1_ct INTEGER, scheduler2_ct INTEGER, template1_ct INTEGER) RETURNS BOOLEAN LANGUAGE PLPGSQL AS
$BODY$
DECLARE
Expand Down Expand Up @@ -602,7 +604,7 @@ SELECT _timescaledb_functions.stop_background_workers();
t
(1 row)

SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE application_name = 'TimescaleDB Background Worker Launcher';
SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE backend_type = 'TimescaleDB Background Worker Launcher';
pg_terminate_backend
----------------------
t
Expand Down
Loading

0 comments on commit e8e94bd

Please sign in to comment.