From c16596002f1dd0487fe586499aec52790b9cf8bb Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Sat, 12 Sep 2026 13:10:01 +0530 Subject: [PATCH v2 1/2] Allow selected PANIC errors to skip core dumps PANIC currently always calls abort(). That is useful for failures that may indicate a server bug, but not necessarily for operational failures such as running out of disk space. A core file can have little diagnostic value in such cases and can worsen disk pressure. Keep PANIC's severity and termination behavior, but add a no_core_dump property to ErrorData. errnocoredump_on_errno() sets it when the errno captured at errstart() matches a caller-selected value. Marked PANICs use _exit(2) instead of abort(), preserving existing postmaster failure handling without running cleanup callbacks. A core-worthy outer PANIC still takes precedence over a nested marked error. Add force_core_dump_on_panic as a developer option to restore abort() for diagnostics. Add injection-point TAP coverage for explicit and promoted PANICs, copied ErrorData, nonmatching and unannotated errors, nested PANICs, and the override. Use the worker I/O method in the test so repeated crash recovery does not exhaust resources belonging to an unrelated I/O method. Discussion: https://postgr.es/m/20231118222911.trznxgya4hvzapva@awork3.anarazel.de --- doc/src/sgml/config.sgml | 23 ++ src/backend/utils/error/elog.c | 37 +++- src/backend/utils/misc/guc_parameters.dat | 7 + src/include/utils/elog.h | 3 + .../injection_points--1.0.sql | 12 ++ .../injection_points/injection_points.c | 200 ++++++++++++++++++ src/test/modules/test_misc/meson.build | 1 + .../modules/test_misc/t/016_panic_no_core.pl | 190 +++++++++++++++++ 8 files changed, 471 insertions(+), 2 deletions(-) create mode 100644 src/test/modules/test_misc/t/016_panic_no_core.pl diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0165eb9ec02..c0fc4293503 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -13546,6 +13546,29 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1) + + force_core_dump_on_panic (boolean) + + force_core_dump_on_panic configuration parameter + + + + + Some PANIC errors are marked to terminate without + requesting a core dump. When this option is set to + on, those errors call + abort() instead, which normally produces a core + dump file. This can be useful for debugging, but repeated failures + can consume significant disk space. Unlike + , this parameter controls + the process reporting the PANIC, not the other + processes terminated by the postmaster afterwards. This parameter + can only be set in the postgresql.conf file or on + the server command line. + + + + debug_logical_replication_streaming (enum) diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index b9d2c96b97a..6cbb5e8a410 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -117,6 +117,7 @@ int Log_destination = LOG_DESTINATION_STDERR; char *Log_destination_string = NULL; bool syslog_sequence_numbers = true; bool syslog_split_messages = true; +bool force_core_dump_on_panic = false; /* Processed form of backtrace_functions GUC */ static char *backtrace_function_list; @@ -487,6 +488,7 @@ errfinish(const char *filename, int lineno, const char *funcname) { ErrorData *edata = &errordata[errordata_stack_depth]; int elevel; + bool no_core_dump; MemoryContext oldcontext; ErrorContextCallback *econtext; @@ -521,6 +523,17 @@ errfinish(const char *filename, int lineno, const char *funcname) econtext = econtext->previous) econtext->callback(econtext->arg); + /* + * Capture this before releasing the current error stack entry below. A + * core-worthy outer PANIC must not be downgraded by a nested error. + */ + no_core_dump = edata->no_core_dump; + for (int i = 0; no_core_dump && i < errordata_stack_depth; i++) + { + if (errordata[i].elevel >= PANIC && !errordata[i].no_core_dump) + no_core_dump = false; + } + /* * If ERROR (not more nor less) we pass it off to the current handler. * Printing it and popping the stack is the responsibility of the handler. @@ -611,13 +624,16 @@ errfinish(const char *filename, int lineno, const char *funcname) if (elevel >= PANIC) { /* - * Serious crash time. Postmaster will observe SIGABRT process exit - * status and kill the other backends too. + * Serious crash time. If this error is marked not to dump core, exit + * without running cleanup callbacks. Exit code 2 makes the postmaster + * treat this as a crash and kill the other backends too. * * XXX: what if we are *in* the postmaster? abort() won't kill our * children... */ fflush(NULL); + if (no_core_dump && !force_core_dump_on_panic) + _exit(2); abort(); } @@ -1652,6 +1668,22 @@ errhidecontext(bool hide_ctx) return 0; /* return value does not matter */ } +/* + * errnocoredump_on_errno --- skip a core dump for the specified errno + */ +int +errnocoredump_on_errno(int errnum) +{ + ErrorData *edata = &errordata[errordata_stack_depth]; + + /* we don't bother incrementing recursion_depth */ + CHECK_STACK_DEPTH(); + + edata->no_core_dump |= edata->saved_errno == errnum; + + return 0; /* return value does not matter */ +} + /* * errposition --- add cursor position to the current error */ @@ -2130,6 +2162,7 @@ ThrowErrorData(ErrorData *edata) newedata->internalpos = edata->internalpos; if (edata->internalquery) newedata->internalquery = pstrdup(edata->internalquery); + newedata->no_core_dump = edata->no_core_dump; MemoryContextSwitchTo(oldcontext); recursion_depth--; diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index 93a434858bf..844c18efd0e 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -1104,6 +1104,13 @@ options => 'file_extend_method_options', }, +{ name => 'force_core_dump_on_panic', type => 'bool', context => 'PGC_SIGHUP', group => 'DEVELOPER_OPTIONS', + short_desc => 'Force core dumps for PANIC errors marked to skip them.', + flags => 'GUC_NOT_IN_SAMPLE', + variable => 'force_core_dump_on_panic', + boot_val => 'false', +}, + { name => 'from_collapse_limit', type => 'int', context => 'PGC_USERSET', group => 'QUERY_TUNING_OTHER', short_desc => 'Sets the FROM-list size beyond which subqueries are not collapsed.', long_desc => 'The planner will merge subqueries into upper queries if the resulting FROM list would have no more than this many items.', diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 6ae376ba001..89d80e4a407 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -219,6 +219,7 @@ extern int errcontext_msg(const char *fmt, ...) pg_attribute_printf(1, 2); extern int errhidestmt(bool hide_stmt); extern int errhidecontext(bool hide_ctx); +extern int errnocoredump_on_errno(int errnum); extern int errbacktrace(void); @@ -422,6 +423,7 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack; pg_re_throw() extern PGDLLIMPORT sigjmp_buf *PG_exception_stack; +extern PGDLLIMPORT bool force_core_dump_on_panic; /* Stuff that error handlers might want to use */ @@ -439,6 +441,7 @@ typedef struct ErrorData bool output_to_client; /* will report to client? */ bool hide_stmt; /* true to prevent STATEMENT: inclusion */ bool hide_ctx; /* true to prevent CONTEXT: inclusion */ + bool no_core_dump; /* true to skip core dump for PANIC */ const char *filename; /* __FILE__ of ereport() call */ int lineno; /* __LINE__ of ereport() call */ const char *funcname; /* __func__ of ereport() call */ diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql index 2efb307f5bf..bf21d94ecf6 100644 --- a/src/test/modules/injection_points/injection_points--1.0.sql +++ b/src/test/modules/injection_points/injection_points--1.0.sql @@ -48,6 +48,18 @@ RETURNS void AS 'MODULE_PATHNAME', 'injection_points_run' LANGUAGE C PARALLEL UNSAFE; +-- Execute an injection point from a critical section. +CREATE FUNCTION injection_points_run_in_critical_section(IN point_name TEXT) +RETURNS void +AS 'MODULE_PATHNAME', 'injection_points_run_in_critical_section' +LANGUAGE C STRICT PARALLEL UNSAFE; + +-- Replace SIGABRT with a distinctive process exit for tests. +CREATE FUNCTION injection_points_intercept_abort() +RETURNS void +AS 'MODULE_PATHNAME', 'injection_points_intercept_abort' +LANGUAGE C STRICT PARALLEL UNSAFE; + -- -- injection_points_cached() -- diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 66d8158d0c2..b6a9a05b6d8 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -17,6 +17,11 @@ #include "postgres.h" +#include +#ifndef WIN32 +#include +#endif + #include "fmgr.h" #include "funcapi.h" #include "injection_points.h" @@ -75,6 +80,21 @@ static InjectionPointSharedState *inj_state = NULL; extern PGDLLEXPORT void injection_error(const char *name, const void *private_data, void *arg); +extern PGDLLEXPORT void injection_error_disk_full(const char *name, + const void *private_data, + void *arg); +extern PGDLLEXPORT void injection_error_io(const char *name, + const void *private_data, + void *arg); +extern PGDLLEXPORT void injection_error_disk_full_rethrow_panic(const char *name, + const void *private_data, + void *arg); +extern PGDLLEXPORT void injection_panic_disk_full(const char *name, + const void *private_data, + void *arg); +extern PGDLLEXPORT void injection_panic_with_nested_disk_full(const char *name, + const void *private_data, + void *arg); extern PGDLLEXPORT void injection_notice(const char *name, const void *private_data, void *arg); @@ -87,12 +107,41 @@ static bool injection_point_local = false; static void injection_shmem_request(void *arg); static void injection_shmem_init(void *arg); +#ifdef WIN32 +static void injection_abort_handler(int signal); +#else +static void injection_abort_handler(SIGNAL_ARGS); +#endif +static void injection_nested_disk_full_panic(void *arg); static const ShmemCallbacks injection_shmem_callbacks = { .request_fn = injection_shmem_request, .init_fn = injection_shmem_init, }; +static void +#ifdef WIN32 +injection_abort_handler(int signal) +#else +injection_abort_handler(SIGNAL_ARGS) +#endif +{ + _exit(42); +} + +static void +injection_nested_disk_full_panic(void *arg) +{ + ErrorContextCallback *callback = arg; + + error_context_stack = callback->previous; + errno = ENOSPC; + ereport(PANIC, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("nested disk-full PANIC"))); +} + /* * Routine for shared memory area initialization, used as a callback * when initializing dynamically with a DSM or when loading the module. @@ -206,6 +255,117 @@ injection_error(const char *name, const void *private_data, void *arg) elog(ERROR, "error triggered for injection point %s", name); } +void +injection_error_disk_full(const char *name, const void *private_data, void *arg) +{ + const InjectionPointCondition *condition = private_data; + char *argstr = arg; + + if (!injection_point_allowed(condition, argstr)) + return; + + errno = ENOSPC; + if (argstr) + ereport(ERROR, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("error triggered for injection point %s (%s)", + name, argstr))); + else + ereport(ERROR, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("error triggered for injection point %s", name))); +} + +void +injection_error_io(const char *name, const void *private_data, void *arg) +{ + const InjectionPointCondition *condition = private_data; + + if (!injection_point_allowed(condition, arg)) + return; + + errno = EIO; + ereport(ERROR, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("I/O error triggered for injection point %s", name))); +} + +void +injection_error_disk_full_rethrow_panic(const char *name, + const void *private_data, void *arg) +{ + const InjectionPointCondition *condition = private_data; + MemoryContext oldcontext = CurrentMemoryContext; + + if (!injection_point_allowed(condition, arg)) + return; + + PG_TRY(); + { + errno = ENOSPC; + ereport(ERROR, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("rethrow disk-full PANIC for injection point %s", name))); + } + PG_CATCH(); + { + ErrorData *edata; + + MemoryContextSwitchTo(oldcontext); + edata = CopyErrorData(); + FlushErrorState(); + edata->elevel = PANIC; + ThrowErrorData(edata); + } + PG_END_TRY(); +} + +void +injection_panic_disk_full(const char *name, const void *private_data, void *arg) +{ + const InjectionPointCondition *condition = private_data; + char *argstr = arg; + + if (!injection_point_allowed(condition, argstr)) + return; + + errno = ENOSPC; + if (argstr) + ereport(PANIC, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("panic triggered for injection point %s (%s)", + name, argstr))); + else + ereport(PANIC, + (errcode_for_file_access(), + errnocoredump_on_errno(ENOSPC), + errmsg("panic triggered for injection point %s", name))); +} + +void +injection_panic_with_nested_disk_full(const char *name, + const void *private_data, void *arg) +{ + const InjectionPointCondition *condition = private_data; + ErrorContextCallback callback; + + if (!injection_point_allowed(condition, arg)) + return; + + callback.previous = error_context_stack; + callback.callback = injection_nested_disk_full_panic; + callback.arg = &callback; + error_context_stack = &callback; + + ereport(PANIC, + (errmsg("outer core-worthy PANIC for injection point %s", name))); +} + void injection_notice(const char *name, const void *private_data, void *arg) { @@ -331,6 +491,16 @@ injection_points_attach(PG_FUNCTION_ARGS) if (strcmp(action, "error") == 0) function = "injection_error"; + else if (strcmp(action, "error_disk_full") == 0) + function = "injection_error_disk_full"; + else if (strcmp(action, "error_io") == 0) + function = "injection_error_io"; + else if (strcmp(action, "error_disk_full_rethrow_panic") == 0) + function = "injection_error_disk_full_rethrow_panic"; + else if (strcmp(action, "panic_disk_full") == 0) + function = "injection_panic_disk_full"; + else if (strcmp(action, "panic_with_nested_disk_full") == 0) + function = "injection_panic_with_nested_disk_full"; else if (strcmp(action, "notice") == 0) function = "injection_notice"; else if (strcmp(action, "wait") == 0) @@ -450,6 +620,36 @@ injection_points_run(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* Trigger an injection point from a critical section. */ +PG_FUNCTION_INFO_V1(injection_points_run_in_critical_section); +Datum +injection_points_run_in_critical_section(PG_FUNCTION_ARGS) +{ + char *name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + + INJECTION_POINT_LOAD(name); + START_CRIT_SECTION(); + INJECTION_POINT_CACHED(name, NULL); + END_CRIT_SECTION(); + + PG_RETURN_VOID(); +} + +/* Make abort() observable without producing a core file. */ +PG_FUNCTION_INFO_V1(injection_points_intercept_abort); +Datum +injection_points_intercept_abort(PG_FUNCTION_ARGS) +{ +#ifdef WIN32 + if (signal(SIGABRT, injection_abort_handler) == SIG_ERR) + elog(ERROR, "could not install SIGABRT handler"); +#else + pqsignal(SIGABRT, injection_abort_handler); +#endif + + PG_RETURN_VOID(); +} + /* * SQL function for triggering an injection point from cache. */ diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index 5d81f5b13be..4c520a339f8 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -24,6 +24,7 @@ tests += { 't/013_temp_obj_multisession.pl', 't/014_log_statement_max_length.pl', 't/015_temp_schema_exit_deferrable.pl', + 't/016_panic_no_core.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/016_panic_no_core.pl b/src/test/modules/test_misc/t/016_panic_no_core.pl new file mode 100644 index 00000000000..3da35c7c50e --- /dev/null +++ b/src/test/modules/test_misc/t/016_panic_no_core.pl @@ -0,0 +1,190 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +plan skip_all => 'Injection points not supported by this build' + unless $ENV{enable_injection_points} eq 'yes'; + +my $node = PostgreSQL::Test::Cluster->new('panic_no_core'); +$node->init; +# Avoid exhausting io_uring resources across repeated crash recovery. +$node->append_conf( + 'postgresql.conf', qq{ +restart_after_crash = on +log_min_messages = warning +io_method = worker +}); +$node->start; + +is($node->safe_psql('postgres', 'SHOW force_core_dump_on_panic'), + 'off', 'core dump override defaults to off'); + +$node->safe_psql( + 'postgres', q{ +CREATE EXTENSION injection_points; +SELECT injection_points_attach('panic-no-core-promoted', 'error_disk_full'); +}); + +my $log_offset = -s $node->logfile; +my ($ret) = $node->psql( + 'postgres', + q{ +SET log_min_messages = panic; +SELECT injection_points_run_in_critical_section('panic-no-core-promoted'); +}); + +isnt($ret, 0, 'promoted no-core PANIC terminates the backend'); + +$node->wait_for_log( + qr/PANIC:.*error triggered for injection point panic-no-core-promoted/, + $log_offset); + +# Exit code 2 proves that abort() was not used. +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 2/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from promoted no-core PANIC'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-no-core-explicit', 'panic_disk_full')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{ +SET log_min_messages = panic; +SELECT injection_points_run('panic-no-core-explicit'); +}); + +isnt($ret, 0, 'explicit no-core PANIC terminates the backend'); +$node->wait_for_log( + qr/PANIC:.*panic triggered for injection point panic-no-core-explicit/, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 2/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from explicit no-core PANIC'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-no-core-rethrow', 'error_disk_full_rethrow_panic')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{SELECT injection_points_run('panic-no-core-rethrow')}); + +isnt($ret, 0, 'rethrown no-core PANIC terminates the backend'); +$node->wait_for_log( + qr/PANIC:.*rethrow disk-full PANIC for injection point panic-no-core-rethrow/, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 2/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from rethrown no-core PANIC'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-core-io', 'error_io')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{ +SELECT injection_points_intercept_abort(); +SELECT injection_points_run_in_critical_section('panic-core-io'); +}); + +isnt($ret, 0, 'non-ENOSPC PANIC terminates the backend via abort'); +$node->wait_for_log( + qr{PANIC:.*I/O error triggered for injection point panic-core-io}, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 42/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from non-ENOSPC PANIC'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-core-nested', 'panic_with_nested_disk_full')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{ +SELECT injection_points_intercept_abort(); +SELECT injection_points_run('panic-core-nested'); +}); + +isnt($ret, 0, 'outer core-worthy PANIC terminates the backend via abort'); +$node->wait_for_log(qr/PANIC:.*nested disk-full PANIC/, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 42/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'core-worthy outer PANIC wins over nested suppression'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-core-promoted', 'error')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{ +SELECT injection_points_intercept_abort(); +SELECT injection_points_run_in_critical_section('panic-core-promoted'); +}); + +isnt($ret, 0, 'unannotated promoted PANIC terminates the backend'); +$node->wait_for_log( + qr/PANIC:.*error triggered for injection point panic-core-promoted/, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 42/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from unannotated promoted PANIC'); + +$node->safe_psql('postgres', + 'ALTER SYSTEM SET force_core_dump_on_panic = on'); +$node->safe_psql('postgres', 'SELECT pg_reload_conf()'); +ok($node->poll_query_until( + 'postgres', + q{SELECT current_setting('force_core_dump_on_panic') = 'on'}), + 'core dump override is active'); + +$node->safe_psql( + 'postgres', + q{SELECT injection_points_attach('panic-core-override', 'panic_disk_full')}); + +$log_offset = -s $node->logfile; +($ret) = $node->psql( + 'postgres', + q{ +SELECT injection_points_intercept_abort(); +SELECT injection_points_run('panic-core-override'); +}); + +isnt($ret, 0, 'core dump override terminates the backend via abort'); +$node->wait_for_log( + qr/PANIC:.*panic triggered for injection point panic-core-override/, + $log_offset); +$node->wait_for_log(qr/\(PID \d+\) exited with exit code 42/, + $log_offset); + +is($node->poll_query_until('postgres', undef, ''), + '1', 'server recovers from overridden no-core PANIC'); + +$node->stop; +done_testing(); -- 2.34.1