From e060390eac9c277b1023038a7d4c6310ba016e5c Mon Sep 17 00:00:00 2001 From: dilipkumar Date: Thu, 16 Jul 2020 11:41:28 +0530 Subject: [PATCH v4] Provide a GUC to allow vacuum to continue on corrupted tuple A new GUC to control whether the vacuum will error out immediately on detection of a corrupted tuple or it will just emit a WARNING for each such instance and complete the rest of the vacuuming. However, nothing will be done to the corrupted tuple itself and we will also not advance the relfrozenxid and relminmxid. --- doc/src/sgml/config.sgml | 22 ++++ src/backend/access/heap/heapam.c | 113 ++++++++++++++++++--- src/backend/access/heap/vacuumlazy.c | 26 ++++- src/backend/commands/vacuum.c | 6 ++ src/backend/utils/misc/guc.c | 9 ++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/access/heapam_xlog.h | 4 +- src/include/commands/vacuum.h | 1 + .../test_misc/t/002_vacuum_tolerate_damage.pl | 49 +++++++++ 9 files changed, 212 insertions(+), 19 deletions(-) create mode 100644 src/test/modules/test_misc/t/002_vacuum_tolerate_damage.pl diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 7a7177c..f3f7b45 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8340,6 +8340,28 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; + + vacuum_tolerate_damage (boolean) + + vacuum_tolerate_damage configuration parameter + + + + + When set to off, which is the default, the VACUUM will raise + a ERROR-level error if detected a corrupted tuple. This causes the operation to + stop immediately. + + + + If set to on, the VACUUM will instead emit a WARNING for each + such tuple but the operation will continue vacuuming the remaining tuples. However, + this will not do anything to the corrupted tuples and it will also not advance the + relfrozenxid and the relminmxid. + + + + bytea_output (enum) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 9b5f417..35b3579 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -52,6 +52,7 @@ #include "access/xloginsert.h" #include "access/xlogutils.h" #include "catalog/catalog.h" +#include "commands/vacuum.h" #include "miscadmin.h" #include "pgstat.h" #include "port/atomics.h" @@ -5802,6 +5803,7 @@ heap_inplace_update(Relation relation, HeapTuple tuple) #define FRM_RETURN_IS_XID 0x0004 #define FRM_RETURN_IS_MULTI 0x0008 #define FRM_MARK_COMMITTED 0x0010 +#define FRM_FOUND_CORRUPTION 0x0020 /* * FreezeMultiXactId @@ -5823,12 +5825,19 @@ heap_inplace_update(Relation relation, HeapTuple tuple) * FRM_RETURN_IS_MULTI * The return value is a new MultiXactId to set as new Xmax. * (caller must obtain proper infomask bits using GetMultiXactIdHintBits) + * FRM_FOUND_CORRUPTION + * This flag will be set if a corrupted multi-xact is detected. + * Ideally, in such cases it will report an error but based on the + * vacuum_damage_elevel, it might just complain about the corruption + * and allow vacuum to continue. So if the flag is set the caller will + * not do anything to this tuple. + * vacuum_damage_elevel - Error level for reporting the corrupted tuple. */ static TransactionId FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, TransactionId relfrozenxid, TransactionId relminmxid, TransactionId cutoff_xid, MultiXactId cutoff_multi, - uint16 *flags) + uint16 *flags, int vacuum_damage_elevel) { TransactionId xid = InvalidTransactionId; int i; @@ -5854,10 +5863,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, return InvalidTransactionId; } else if (MultiXactIdPrecedes(multi, relminmxid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found multixact %u from before relminmxid %u", multi, relminmxid))); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } else if (MultiXactIdPrecedes(multi, cutoff_multi)) { /* @@ -5868,10 +5881,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, */ if (MultiXactIdIsRunning(multi, HEAP_XMAX_IS_LOCKED_ONLY(t_infomask))) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("multixact %u from before cutoff %u found to be still running", multi, cutoff_multi))); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } if (HEAP_XMAX_IS_LOCKED_ONLY(t_infomask)) { @@ -5887,10 +5904,14 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, Assert(TransactionIdIsValid(xid)); if (TransactionIdPrecedes(xid, relfrozenxid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found update xid %u from before relfrozenxid %u", xid, relfrozenxid))); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } /* * If the xid is older than the cutoff, it has to have aborted, @@ -5899,9 +5920,13 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, if (TransactionIdPrecedes(xid, cutoff_xid)) { if (TransactionIdDidCommit(xid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("cannot freeze committed update xid %u", xid))); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } *flags |= FRM_INVALIDATE_XMAX; xid = InvalidTransactionId; /* not strictly necessary */ } @@ -5975,10 +6000,15 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, Assert(TransactionIdIsValid(xid)); if (TransactionIdPrecedes(xid, relfrozenxid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found update xid %u from before relfrozenxid %u", xid, relfrozenxid))); + pfree(members); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } /* * It's an update; should we keep it? If the transaction is known @@ -6025,10 +6055,15 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, */ if (TransactionIdIsValid(update_xid) && TransactionIdPrecedes(update_xid, cutoff_xid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found update xid %u from before xid cutoff %u", update_xid, cutoff_xid))); + pfree(members); + *flags |= FRM_FOUND_CORRUPTION; + return InvalidTransactionId; + } /* * If we determined that it's an Xid corresponding to an update @@ -6110,6 +6145,11 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask, * HeapTupleSatisfiesVacuum() and determined that it is not HEAPTUPLE_DEAD * (else we should be removing the tuple, not freezing it). * + * vacuum_damage_elevel - Error level for reporting the corrupted tuple. + * found_corruption - Out parameter, must be a valid pointer if the + * vacuum_damage_elevel is set to the WARNING. This will be set to true + * if any corrupted tuple is found. + * * NB: cutoff_xid *must* be <= the current global xmin, to ensure that any * XID older than it could neither be running nor seen as running by any * open transaction. This ensures that the replacement will not change @@ -6128,7 +6168,8 @@ bool heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId relfrozenxid, TransactionId relminmxid, TransactionId cutoff_xid, TransactionId cutoff_multi, - xl_heap_freeze_tuple *frz, bool *totally_frozen_p) + xl_heap_freeze_tuple *frz, bool *totally_frozen_p, + bool *found_corruption, int vacuum_damage_elevel) { bool changed = false; bool xmax_already_frozen = false; @@ -6141,6 +6182,17 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, frz->t_infomask = tuple->t_infomask; frz->xmax = HeapTupleHeaderGetRawXmax(tuple); + *totally_frozen_p = false; + + /* + * The vacuum_damage_elevel must be set to either ERROR or WARNING and + * if it is set to WARNING then the caller must pass a valid pointer for + * found_corruption. + */ + Assert((vacuum_damage_elevel == WARNING) || + (vacuum_damage_elevel == ERROR)); + Assert((found_corruption != NULL) || (vacuum_damage_elevel == ERROR)); + /* * Process xmin. xmin_frozen has two slightly different meanings: in the * !XidIsNormal case, it means "the xmin doesn't need any freezing" (it's @@ -6155,19 +6207,27 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, else { if (TransactionIdPrecedes(xid, relfrozenxid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found xmin %u from before relfrozenxid %u", xid, relfrozenxid))); + *found_corruption = true; + return false; + } xmin_frozen = TransactionIdPrecedes(xid, cutoff_xid); if (xmin_frozen) { if (!TransactionIdDidCommit(xid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("uncommitted xmin %u from before xid cutoff %u needs to be frozen", xid, cutoff_xid))); + *found_corruption = true; + return false; + } frz->t_infomask |= HEAP_XMIN_FROZEN; changed = true; @@ -6192,7 +6252,17 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, newxmax = FreezeMultiXactId(xid, tuple->t_infomask, relfrozenxid, relminmxid, - cutoff_xid, cutoff_multi, &flags); + cutoff_xid, cutoff_multi, + &flags, vacuum_damage_elevel); + /* + * If we have detected a corrupted multi-xact id then just set the + * found_corruption flag and return. + */ + if (flags & FRM_FOUND_CORRUPTION) + { + *found_corruption = true; + return false; + } freeze_xmax = (flags & FRM_INVALIDATE_XMAX); @@ -6236,10 +6306,14 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, else if (TransactionIdIsNormal(xid)) { if (TransactionIdPrecedes(xid, relfrozenxid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found xmax %u from before relfrozenxid %u", xid, relfrozenxid))); + *found_corruption = true; + return false; + } if (TransactionIdPrecedes(xid, cutoff_xid)) { @@ -6251,10 +6325,14 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, */ if (!HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask) && TransactionIdDidCommit(xid)) - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("cannot freeze committed xmax %u", xid))); + *found_corruption = true; + return false; + } freeze_xmax = true; } else @@ -6267,10 +6345,14 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, xmax_already_frozen = true; } else - ereport(ERROR, + { + ereport(vacuum_damage_elevel, (errcode(ERRCODE_DATA_CORRUPTED), errmsg_internal("found xmax %u (infomask 0x%04x) not frozen, not multi, not normal", xid, tuple->t_infomask))); + *found_corruption = true; + return false; + } if (freeze_xmax) { @@ -6386,7 +6468,8 @@ heap_freeze_tuple(HeapTupleHeader tuple, do_freeze = heap_prepare_freeze_tuple(tuple, relfrozenxid, relminmxid, cutoff_xid, cutoff_multi, - &frz, &tuple_totally_frozen); + &frz, &tuple_totally_frozen, + NULL, ERROR); /* * Note that because this is not a WAL-logged operation, we don't need to diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index a0da444..6f848ea 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -312,6 +312,7 @@ typedef struct LVRelStats int num_index_scans; TransactionId latestRemovedXid; bool lock_waiter_detected; + bool corrupted_tuple_detected; /* Used for error callback */ char *indname; @@ -497,6 +498,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, vacrelstats->num_index_scans = 0; vacrelstats->pages_removed = 0; vacrelstats->lock_waiter_detected = false; + vacrelstats->corrupted_tuple_detected = false; /* Open all indexes of the relation */ vac_open_indexes(onerel, RowExclusiveLock, &nindexes, &Irel); @@ -597,8 +599,20 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, if (new_rel_allvisible > new_rel_pages) new_rel_allvisible = new_rel_pages; - new_frozen_xid = scanned_all_unfrozen ? FreezeLimit : InvalidTransactionId; - new_min_multi = scanned_all_unfrozen ? MultiXactCutoff : InvalidMultiXactId; + /* + * Don't advance the relfrozenxid and relminmxid, if we have detected a + * corrupted tuple. + */ + if (scanned_all_unfrozen && !vacrelstats->corrupted_tuple_detected) + { + new_frozen_xid = FreezeLimit; + new_min_multi = MultiXactCutoff; + } + else + { + new_frozen_xid = InvalidTransactionId; + new_min_multi = InvalidMultiXactId; + } vac_update_relstats(onerel, new_rel_pages, @@ -1460,6 +1474,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, else { bool tuple_totally_frozen; + bool found_corruption = false; num_tuples += 1; hastup = true; @@ -1472,8 +1487,13 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, relfrozenxid, relminmxid, FreezeLimit, MultiXactCutoff, &frozen[nfrozen], - &tuple_totally_frozen)) + &tuple_totally_frozen, + &found_corruption, + vacuum_tolerate_damage ? + WARNING : ERROR)) frozen[nfrozen++].offset = offnum; + else if (found_corruption) + vacrelstats->corrupted_tuple_detected = true; if (!tuple_totally_frozen) all_frozen = false; diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 23eb605..401a45f 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -63,6 +63,12 @@ int vacuum_freeze_table_age; int vacuum_multixact_freeze_min_age; int vacuum_multixact_freeze_table_age; +/* + * Whether to continue with the vacuuming after detecting a corrupted tuple. + * If this flag is set then on detection of a corrupted tuple during a freeze, + * it will report each such occurrence with WARNING and continue the vacuum. + */ +bool vacuum_tolerate_damage = false; /* A few variables that don't seem worth passing around as parameters */ static MemoryContext vac_context = NULL; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index de87ad6..c7a9eb3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2036,6 +2036,15 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"vacuum_tolerate_damage", PGC_USERSET, ERROR_HANDLING_OPTIONS, + gettext_noop("Whether to continue running vacuum after detecting corrupted tuple."), + }, + &vacuum_tolerate_damage, + false, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 9cb571f..a04d6aa 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -671,6 +671,7 @@ #vacuum_cleanup_index_scale_factor = 0.1 # fraction of total number of tuples # before index cleanup, 0 always performs # index cleanup +#vacuum_tolerate_damage = false #bytea_output = 'hex' # hex, escape #xmlbinary = 'base64' #xmloption = 'content' diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index aa17f7d..fb0bb31 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -412,7 +412,9 @@ extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid, TransactionId cutoff_multi, xl_heap_freeze_tuple *frz, - bool *totally_frozen); + bool *totally_frozen, + bool *found_corruption, + int vacuum_damage_elevel); extern void heap_execute_freeze_tuple(HeapTupleHeader tuple, xl_heap_freeze_tuple *xlrec_tp); extern XLogRecPtr log_heap_visible(RelFileNode rnode, Buffer heap_buffer, diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index a4cd721..322fa17 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -237,6 +237,7 @@ extern int vacuum_freeze_min_age; extern int vacuum_freeze_table_age; extern int vacuum_multixact_freeze_min_age; extern int vacuum_multixact_freeze_table_age; +extern bool vacuum_tolerate_damage; /* Variables for cost-based parallel vacuum */ extern pg_atomic_uint32 *VacuumSharedCostBalance; diff --git a/src/test/modules/test_misc/t/002_vacuum_tolerate_damage.pl b/src/test/modules/test_misc/t/002_vacuum_tolerate_damage.pl new file mode 100644 index 0000000..acf1266 --- /dev/null +++ b/src/test/modules/test_misc/t/002_vacuum_tolerate_damage.pl @@ -0,0 +1,49 @@ +# Verify the vacuum_tolerate_damage GUC + +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests => 2; + +# Initialize a test cluster +my $node = get_new_node('primary'); +$node->init(); +$node->start; + +# Run a SQL command and return psql's stderr +sub run_sql_command +{ + my $sql = shift; + my $stderr; + + $node->psql( + 'postgres', + $sql, + stderr => \$stderr); + return $stderr; +} + +my $output; + +# Insert 2 tuple in the table and update the relfrozenxid for the table to +# the future xid. +run_sql_command( + "create table test_vac (a int) WITH (autovacuum_enabled = off); + insert into test_vac values (1), (2); + update pg_class set relfrozenxid=txid_current()::text::xid where relname='test_vac';"); + +$output = run_sql_command('vacuum(freeze, disable_page_skipping) test_vac;'); +ok( $output =~ m/ERROR:.*found xmin.*before relfrozenxid/); + +# set the vacuum_tolerate_damage and run again +$output = run_sql_command('set vacuum_tolerate_damage=true; + vacuum(freeze, disable_page_skipping) test_vac;'); + + +# this time we should get WARNING for both the tuples +ok( scalar( @{[ $output=~/WARNING:.*found xmin.*before relfrozenxid/gi ]}) == 2); + +run_sql_command('DROP TABLE test_vac;'); + +$node->stop('fast'); -- 1.8.3.1