From cee8b66b68f68bb2acc63185031463dd4f570b09 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 24 Sep 2026 15:59:55 -0400 Subject: [PATCH v1 07/11] Make zero_damaged_pages selectable per relation fork zero_damaged_pages was a boolean that, when on, zeroed a damaged page of any relation fork. Recovering from a torn or corrupt auxiliary page (for example a visibility map page) therefore required disarming the protection for the table's main data as well. Turn it into a comma-separated list of forks -- "main", "fsm", "vm", "init", or "all" -- for which a damaged page header is zeroed (with a warning) instead of raising an error. For backward compatibility a boolean value is still accepted on its own: "on" selects every fork and "off" none, and existing settings keep working. This lets an operator recover from, say, a corrupt visibility map page with zero_damaged_pages = 'vm' while a corrupt heap page still errors out. This will make it more defensible to stop reading the VM with RBM_ZERO_ON_ERROR in redo. --- doc/src/sgml/config.sgml | 35 ++++++++++---- src/backend/storage/buffer/bufmgr.c | 57 ++++++++++++++++++++++- src/backend/storage/smgr/md.c | 2 +- src/backend/utils/misc/guc_parameters.dat | 14 +++--- src/include/storage/bufmgr.h | 3 +- src/include/utils/guc_hooks.h | 3 ++ src/test/modules/test_aio/test_aio.c | 2 +- 7 files changed, 96 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0165eb9ec02..36edb226ccb 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -13333,7 +13333,7 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1) - zero_damaged_pages (boolean) + zero_damaged_pages (string) zero_damaged_pages configuration parameter @@ -13342,18 +13342,35 @@ LOG: CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1) Detection of a damaged page header normally causes PostgreSQL to report an error, aborting the current - transaction. Setting zero_damaged_pages to on causes - the system to instead report a warning, zero out the damaged - page in memory, and continue processing. This behavior will destroy data, - namely all the rows on the damaged page. However, it does allow you to get + transaction. This parameter is a comma-separated list of the relation + forks for which the system instead reports a warning, zeroes out the + damaged page in memory, and continues processing. The recognized fork + names are main (the table or index data), + fsm (the free space map), vm + (the visibility map), and init (the init fork). + all selects every fork. The server always zeroes + damaged free space map pages regardless of this setting, and it does + not normally read init fork pages through shared buffers, so + fsm and init only affect + functions that read a specific fork directly, such as + 's get_raw_page(). + For backward compatibility, on (every fork) and + off (no fork, the default) are also accepted, but + only on their own. + + + Zeroing a page will destroy data, namely all the + rows on the damaged page. However, it does allow you to get past the error and retrieve rows from any undamaged pages that might be present in the table. It is useful for recovering data if corruption has occurred due to a hardware or software error. You should - generally not set this on until you have given up hope of recovering - data from the damaged pages of a table. Zeroed-out pages are not + generally not enable this for a fork until you have given up hope of + recovering data from the damaged pages of a table. Scoping the setting + to a single fork, such as vm, lets you recover from a + damaged auxiliary page without also disarming this protection for the + table's main data. Zeroed-out pages are not forced to disk so it is recommended to recreate the table or - the index before turning this parameter off again. The - default setting is off. + the index before turning this parameter off again. Only superusers and users with the appropriate SET privilege can change this setting. diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 5c82865a084..27539b564f0 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -46,6 +46,7 @@ #include "catalog/storage.h" #include "catalog/storage_xlog.h" #include "common/hashfn.h" +#include "common/relpath.h" #include "executor/instrument.h" #include "lib/binaryheap.h" #include "miscadmin.h" @@ -64,6 +65,8 @@ #include "storage/read_stream.h" #include "storage/smgr.h" #include "storage/standby.h" +#include "utils/guc.h" +#include "utils/guc_hooks.h" #include "utils/memdebug.h" #include "utils/ps_status.h" #include "utils/rel.h" @@ -186,7 +189,15 @@ typedef struct SMgrSortArray } SMgrSortArray; /* GUC variables */ -bool zero_damaged_pages = false; + +/* + * zero_damaged_pages is a list of relation forks for which a damaged page + * header is zeroed (with a warning) instead of raising an error. The raw + * GUC string is parsed into zero_damaged_pages_forks, a bitmask indexed by + * ForkNumber (1 << forknum). + */ +char *zero_damaged_pages_string; +int zero_damaged_pages_forks = 0; int bgwriter_lru_maxpages = 100; double bgwriter_lru_multiplier = 2.0; bool track_io_timing = false; @@ -1987,7 +1998,7 @@ AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress) * zero_damaged_pages, so we can report different log levels / error codes * for zero_damaged_pages and ZERO_ON_ERROR. */ - if (zero_damaged_pages) + if (zero_damaged_pages_forks & (1 << forknum)) flags |= READ_BUFFERS_ZERO_ON_ERROR; /* @@ -9005,3 +9016,45 @@ const PgAioHandleCallbacks aio_local_buffer_readv_cb = { .complete_local = local_buffer_readv_complete, .report = buffer_readv_report, }; + +/* Fork bitmask selecting every relation fork */ +#define ZERO_DAMAGED_PAGES_ALL_FORKS ((1 << (MAX_FORKNUM + 1)) - 1) + +/* + * GUC check_hook for zero_damaged_pages. + * + * The value is a comma-separated list of relation fork names ("main", "fsm", + * "vm", "init"), or "all", for which a damaged page header is zeroed instead + * of raising an error. The resulting fork bitmask is stashed in *extra for + * the assign hook. + * + * Prior to PostgreSQL 20, zero_damaged_pages was a boolean GUC; a boolean + * value meaning true selects every fork. + */ +bool +check_zero_damaged_pages(char **newval, void **extra, GucSource source) +{ + static const struct config_enum_entry options[] = { + {"main", 1 << MAIN_FORKNUM}, + {"fsm", 1 << FSM_FORKNUM}, + {"vm", 1 << VISIBILITYMAP_FORKNUM}, + {"init", 1 << INIT_FORKNUM}, + {"all", ZERO_DAMAGED_PAGES_ALL_FORKS}, + {NULL, 0} + }; + + StaticAssertDecl(lengthof(options) == MAX_FORKNUM + 3, + "zero_damaged_pages must accept every fork name"); + + return check_flag_list_guc(newval, extra, "zero_damaged_pages", options, + true, ZERO_DAMAGED_PAGES_ALL_FORKS); +} + +/* + * GUC assign_hook for zero_damaged_pages. + */ +void +assign_zero_damaged_pages(const char *newval, void *extra) +{ + zero_damaged_pages_forks = *((int *) extra); +} diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 780c88c0630..9e3f15c347f 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -951,7 +951,7 @@ mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, * continuing to work in production builds). Afterwards we * plan to remove this code entirely. */ - if (zero_damaged_pages || InRecovery) + if ((zero_damaged_pages_forks & (1 << forknum)) || InRecovery) { Assert(false); /* see comment above */ diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index c57441f7d98..22906b020ba 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -3669,12 +3669,14 @@ options => 'xmloption_options', }, -{ name => 'zero_damaged_pages', type => 'bool', context => 'PGC_SUSET', group => 'DEVELOPER_OPTIONS', - short_desc => 'Continues processing past damaged page headers.', - long_desc => 'Detection of a damaged page header normally causes PostgreSQL to report an error, aborting the current transaction. Setting "zero_damaged_pages" to true causes the system to instead report a warning, zero out the damaged page, and continue processing. This behavior will destroy data, namely all the rows on the damaged page.', - flags => 'GUC_NOT_IN_SAMPLE', - variable => 'zero_damaged_pages', - boot_val => 'false', +{ name => 'zero_damaged_pages', type => 'string', context => 'PGC_SUSET', group => 'DEVELOPER_OPTIONS', + short_desc => 'Continues processing past damaged page headers for the listed relation forks.', + long_desc => 'Detection of a damaged page header normally causes PostgreSQL to report an error, aborting the current transaction. This is a comma-separated list of relation forks ("main", "fsm", "vm", "init") for which the system instead reports a warning, zeroes out the damaged page, and continues processing; this destroys all the rows on the damaged page. "all" selects every fork. For backward compatibility, "on" (every fork) and "off" (no fork) are also accepted, but only on their own.', + flags => 'GUC_LIST_INPUT | GUC_NOT_IN_SAMPLE', + variable => 'zero_damaged_pages_string', + boot_val => '""', + check_hook => 'check_zero_damaged_pages', + assign_hook => 'assign_zero_damaged_pages', }, ] diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 6837b35fc6d..7d6106ffd7e 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -162,7 +162,8 @@ typedef struct WritebackContext WritebackContext; extern PGDLLIMPORT int NBuffers; /* in bufmgr.c */ -extern PGDLLIMPORT bool zero_damaged_pages; +extern PGDLLIMPORT char *zero_damaged_pages_string; +extern PGDLLIMPORT int zero_damaged_pages_forks; extern PGDLLIMPORT int bgwriter_lru_maxpages; extern PGDLLIMPORT double bgwriter_lru_multiplier; extern PGDLLIMPORT bool track_io_timing; diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 06453a18c03..5677df7dcf9 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -177,5 +177,8 @@ extern bool check_synchronized_standby_slots(char **newval, void **extra, extern void assign_synchronized_standby_slots(const char *newval, void *extra); extern bool check_log_min_messages(char **newval, void **extra, GucSource source); extern void assign_log_min_messages(const char *newval, void *extra); +extern bool check_zero_damaged_pages(char **newval, void **extra, + GucSource source); +extern void assign_zero_damaged_pages(const char *newval, void *extra); #endif /* GUC_HOOKS_H */ diff --git a/src/test/modules/test_aio/test_aio.c b/src/test/modules/test_aio/test_aio.c index 39d857557cf..083c25a9798 100644 --- a/src/test/modules/test_aio/test_aio.c +++ b/src/test/modules/test_aio/test_aio.c @@ -434,7 +434,7 @@ read_rel_block_ll(PG_FUNCTION_ARGS) pgaio_io_set_handle_data_32(ioh, (uint32 *) bufs, nblocks); - if (zero_on_error | zero_damaged_pages) + if (zero_on_error || (zero_damaged_pages_forks & (1 << MAIN_FORKNUM))) srb_flags |= READ_BUFFERS_ZERO_ON_ERROR; if (ignore_checksum_failure) srb_flags |= READ_BUFFERS_IGNORE_CHECKSUM_FAILURES; -- 2.43.0