From f691f5b80a85c66d715b4340ffabb503eb19393e Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 5 Sep 2023 18:26:12 +1200 Subject: [PATCH] Remove the "snapshot too old" feature. Remove the old_snapshot_threshold setting and mechanism for producing the error "snapshot too old", originally added by commit 848ef42b. Unfortunately it had a number of known problems in terms of correctness and performance, mostly reported by Andres in the course of his work on snapshot scalability. We agreed to remove it, after a long period without an active plan to fix it. This is certainly a desirable feature, and someone might propose a new or improved implementation in the future. Reported-by: Andres Freund Discussion: https://postgr.es/m/CACG%3DezYV%2BEvO135fLRdVn-ZusfVsTY6cH1OZqWtezuEYH6ciQA%40mail.gmail.com Discussion: https://postgr.es/m/20200401064008.qob7bfnnbu4w5cw4%40alap3.anarazel.de Discussion: https://postgr.es/m/CA%2BTgmoY%3Daqf0zjTD%2B3dUWYkgMiNDegDLFjo%2B6ze%3DWtpik%2B3XqA%40mail.gmail.com --- contrib/Makefile | 1 - contrib/bloom/blscan.c | 1 - contrib/meson.build | 1 - contrib/old_snapshot/Makefile | 21 - contrib/old_snapshot/meson.build | 23 - contrib/old_snapshot/old_snapshot--1.0.sql | 14 - contrib/old_snapshot/old_snapshot.control | 5 - contrib/old_snapshot/time_mapping.c | 142 ------ doc/src/sgml/config.sgml | 69 --- doc/src/sgml/contrib.sgml | 1 - doc/src/sgml/filelist.sgml | 1 - doc/src/sgml/oldsnapshot.sgml | 33 -- src/backend/access/brin/brin_revmap.c | 7 - src/backend/access/gin/ginbtree.c | 2 - src/backend/access/gin/ginget.c | 4 - src/backend/access/gist/gistget.c | 1 - src/backend/access/hash/hashsearch.c | 6 - src/backend/access/heap/heapam.c | 9 - src/backend/access/heap/pruneheap.c | 120 +---- src/backend/access/heap/vacuumlazy.c | 5 +- src/backend/access/nbtree/nbtsearch.c | 9 - src/backend/access/spgist/spgscan.c | 1 - src/backend/catalog/index.c | 28 +- src/backend/commands/vacuum.c | 19 - src/backend/storage/buffer/bufmgr.c | 17 - src/backend/storage/ipc/ipci.c | 2 - src/backend/storage/ipc/procarray.c | 36 +- src/backend/storage/lmgr/lwlocknames.txt | 2 +- .../utils/activity/wait_event_names.txt | 1 - src/backend/utils/errcodes.txt | 4 - src/backend/utils/misc/guc_tables.c | 11 - src/backend/utils/misc/postgresql.conf.sample | 2 - src/backend/utils/time/snapmgr.c | 468 ------------------ src/include/access/heapam.h | 2 - src/include/storage/bufmgr.h | 36 -- src/include/utils/old_snapshot.h | 75 --- src/include/utils/snapmgr.h | 49 -- src/test/modules/Makefile | 1 - src/test/modules/meson.build | 1 - src/test/modules/snapshot_too_old/.gitignore | 3 - src/test/modules/snapshot_too_old/Makefile | 28 -- .../expected/sto_using_cursor.out | 19 - .../expected/sto_using_hash_index.out | 19 - .../expected/sto_using_select.out | 18 - src/test/modules/snapshot_too_old/meson.build | 19 - .../specs/sto_using_cursor.spec | 38 -- .../specs/sto_using_hash_index.spec | 31 -- .../specs/sto_using_select.spec | 37 -- src/test/modules/snapshot_too_old/sto.conf | 2 - src/tools/pgindent/typedefs.list | 2 - 50 files changed, 21 insertions(+), 1425 deletions(-) delete mode 100644 contrib/old_snapshot/Makefile delete mode 100644 contrib/old_snapshot/meson.build delete mode 100644 contrib/old_snapshot/old_snapshot--1.0.sql delete mode 100644 contrib/old_snapshot/old_snapshot.control delete mode 100644 contrib/old_snapshot/time_mapping.c delete mode 100644 doc/src/sgml/oldsnapshot.sgml delete mode 100644 src/include/utils/old_snapshot.h delete mode 100644 src/test/modules/snapshot_too_old/.gitignore delete mode 100644 src/test/modules/snapshot_too_old/Makefile delete mode 100644 src/test/modules/snapshot_too_old/expected/sto_using_cursor.out delete mode 100644 src/test/modules/snapshot_too_old/expected/sto_using_hash_index.out delete mode 100644 src/test/modules/snapshot_too_old/expected/sto_using_select.out delete mode 100644 src/test/modules/snapshot_too_old/meson.build delete mode 100644 src/test/modules/snapshot_too_old/specs/sto_using_cursor.spec delete mode 100644 src/test/modules/snapshot_too_old/specs/sto_using_hash_index.spec delete mode 100644 src/test/modules/snapshot_too_old/specs/sto_using_select.spec delete mode 100644 src/test/modules/snapshot_too_old/sto.conf diff --git a/contrib/Makefile b/contrib/Makefile index bbf220407b0..da4e2316a3b 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -29,7 +29,6 @@ SUBDIRS = \ lo \ ltree \ oid2name \ - old_snapshot \ pageinspect \ passwordcheck \ pg_buffercache \ diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c index 6cc7d07164a..61d1f66b387 100644 --- a/contrib/bloom/blscan.c +++ b/contrib/bloom/blscan.c @@ -132,7 +132,6 @@ blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm) LockBuffer(buffer, BUFFER_LOCK_SHARE); page = BufferGetPage(buffer); - TestForOldSnapshot(scan->xs_snapshot, scan->indexRelation, page); if (!PageIsNew(page) && !BloomPageIsDeleted(page)) { diff --git a/contrib/meson.build b/contrib/meson.build index bd4a57c43c0..84d4e185618 100644 --- a/contrib/meson.build +++ b/contrib/meson.build @@ -37,7 +37,6 @@ subdir('lo') subdir('ltree') subdir('ltree_plpython') subdir('oid2name') -subdir('old_snapshot') subdir('pageinspect') subdir('passwordcheck') subdir('pg_buffercache') diff --git a/contrib/old_snapshot/Makefile b/contrib/old_snapshot/Makefile deleted file mode 100644 index adb557532fc..00000000000 --- a/contrib/old_snapshot/Makefile +++ /dev/null @@ -1,21 +0,0 @@ -# contrib/old_snapshot/Makefile - -MODULE_big = old_snapshot -OBJS = \ - $(WIN32RES) \ - time_mapping.o - -EXTENSION = old_snapshot -DATA = old_snapshot--1.0.sql -PGFILEDESC = "old_snapshot - utilities in support of old_snapshot_threshold" - -ifdef USE_PGXS -PG_CONFIG = pg_config -PGXS := $(shell $(PG_CONFIG) --pgxs) -include $(PGXS) -else -subdir = contrib/old_snapshot -top_builddir = ../.. -include $(top_builddir)/src/Makefile.global -include $(top_srcdir)/contrib/contrib-global.mk -endif diff --git a/contrib/old_snapshot/meson.build b/contrib/old_snapshot/meson.build deleted file mode 100644 index fe5fb9027ab..00000000000 --- a/contrib/old_snapshot/meson.build +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright (c) 2022-2023, PostgreSQL Global Development Group - -old_snapshot_sources = files( - 'time_mapping.c', -) - -if host_system == 'windows' - old_snapshot_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ - '--NAME', 'old_snapshot', - '--FILEDESC', 'old_snapshot - utilities in support of old_snapshot_threshold',]) -endif - -old_snapshot = shared_module('old_snapshot', - old_snapshot_sources, - kwargs: contrib_mod_args, -) -contrib_targets += old_snapshot - -install_data( - 'old_snapshot.control', - 'old_snapshot--1.0.sql', - kwargs: contrib_data_args, -) diff --git a/contrib/old_snapshot/old_snapshot--1.0.sql b/contrib/old_snapshot/old_snapshot--1.0.sql deleted file mode 100644 index 9ebb8829e37..00000000000 --- a/contrib/old_snapshot/old_snapshot--1.0.sql +++ /dev/null @@ -1,14 +0,0 @@ -/* contrib/old_snapshot/old_snapshot--1.0.sql */ - --- complain if script is sourced in psql, rather than via CREATE EXTENSION -\echo Use "CREATE EXTENSION old_snapshot" to load this file. \quit - --- Show visibility map and page-level visibility information for each block. -CREATE FUNCTION pg_old_snapshot_time_mapping(array_offset OUT int4, - end_timestamp OUT timestamptz, - newest_xmin OUT xid) -RETURNS SETOF record -AS 'MODULE_PATHNAME', 'pg_old_snapshot_time_mapping' -LANGUAGE C STRICT; - --- XXX. Do we want REVOKE commands here? diff --git a/contrib/old_snapshot/old_snapshot.control b/contrib/old_snapshot/old_snapshot.control deleted file mode 100644 index 491eec536cd..00000000000 --- a/contrib/old_snapshot/old_snapshot.control +++ /dev/null @@ -1,5 +0,0 @@ -# old_snapshot extension -comment = 'utilities in support of old_snapshot_threshold' -default_version = '1.0' -module_pathname = '$libdir/old_snapshot' -relocatable = true diff --git a/contrib/old_snapshot/time_mapping.c b/contrib/old_snapshot/time_mapping.c deleted file mode 100644 index 352308cd49b..00000000000 --- a/contrib/old_snapshot/time_mapping.c +++ /dev/null @@ -1,142 +0,0 @@ -/*------------------------------------------------------------------------- - * - * time_mapping.c - * time to XID mapping information - * - * Copyright (c) 2020-2023, PostgreSQL Global Development Group - * - * contrib/old_snapshot/time_mapping.c - *------------------------------------------------------------------------- - */ -#include "postgres.h" - -#include "funcapi.h" -#include "storage/lwlock.h" -#include "utils/old_snapshot.h" -#include "utils/snapmgr.h" -#include "utils/timestamp.h" - -/* - * Backend-private copy of the information from oldSnapshotControl which relates - * to the time to XID mapping, plus an index so that we can iterate. - * - * Note that the length of the xid_by_minute array is given by - * OLD_SNAPSHOT_TIME_MAP_ENTRIES (which is not a compile-time constant). - */ -typedef struct -{ - int current_index; - int head_offset; - TimestampTz head_timestamp; - int count_used; - TransactionId xid_by_minute[FLEXIBLE_ARRAY_MEMBER]; -} OldSnapshotTimeMapping; - -#define NUM_TIME_MAPPING_COLUMNS 3 - -PG_MODULE_MAGIC; -PG_FUNCTION_INFO_V1(pg_old_snapshot_time_mapping); - -static OldSnapshotTimeMapping *GetOldSnapshotTimeMapping(void); -static HeapTuple MakeOldSnapshotTimeMappingTuple(TupleDesc tupdesc, - OldSnapshotTimeMapping *mapping); - -/* - * SQL-callable set-returning function. - */ -Datum -pg_old_snapshot_time_mapping(PG_FUNCTION_ARGS) -{ - FuncCallContext *funcctx; - OldSnapshotTimeMapping *mapping; - - if (SRF_IS_FIRSTCALL()) - { - MemoryContext oldcontext; - TupleDesc tupdesc; - - funcctx = SRF_FIRSTCALL_INIT(); - oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); - mapping = GetOldSnapshotTimeMapping(); - funcctx->user_fctx = mapping; - if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) - elog(ERROR, "return type must be a row type"); - funcctx->tuple_desc = tupdesc; - MemoryContextSwitchTo(oldcontext); - } - - funcctx = SRF_PERCALL_SETUP(); - mapping = (OldSnapshotTimeMapping *) funcctx->user_fctx; - - while (mapping->current_index < mapping->count_used) - { - HeapTuple tuple; - - tuple = MakeOldSnapshotTimeMappingTuple(funcctx->tuple_desc, mapping); - ++mapping->current_index; - SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple)); - } - - SRF_RETURN_DONE(funcctx); -} - -/* - * Get the old snapshot time mapping data from shared memory. - */ -static OldSnapshotTimeMapping * -GetOldSnapshotTimeMapping(void) -{ - OldSnapshotTimeMapping *mapping; - - mapping = palloc(offsetof(OldSnapshotTimeMapping, xid_by_minute) - + sizeof(TransactionId) * OLD_SNAPSHOT_TIME_MAP_ENTRIES); - mapping->current_index = 0; - - LWLockAcquire(OldSnapshotTimeMapLock, LW_SHARED); - mapping->head_offset = oldSnapshotControl->head_offset; - mapping->head_timestamp = oldSnapshotControl->head_timestamp; - mapping->count_used = oldSnapshotControl->count_used; - for (int i = 0; i < OLD_SNAPSHOT_TIME_MAP_ENTRIES; ++i) - mapping->xid_by_minute[i] = oldSnapshotControl->xid_by_minute[i]; - LWLockRelease(OldSnapshotTimeMapLock); - - return mapping; -} - -/* - * Convert one entry from the old snapshot time mapping to a HeapTuple. - */ -static HeapTuple -MakeOldSnapshotTimeMappingTuple(TupleDesc tupdesc, OldSnapshotTimeMapping *mapping) -{ - Datum values[NUM_TIME_MAPPING_COLUMNS]; - bool nulls[NUM_TIME_MAPPING_COLUMNS]; - int array_position; - TimestampTz timestamp; - - /* - * Figure out the array position corresponding to the current index. - * - * Index 0 means the oldest entry in the mapping, which is stored at - * mapping->head_offset. Index 1 means the next-oldest entry, which is a - * the following index, and so on. We wrap around when we reach the end of - * the array. - */ - array_position = (mapping->head_offset + mapping->current_index) - % OLD_SNAPSHOT_TIME_MAP_ENTRIES; - - /* - * No explicit timestamp is stored for any entry other than the oldest - * one, but each entry corresponds to 1-minute period, so we can just add. - */ - timestamp = TimestampTzPlusMilliseconds(mapping->head_timestamp, - mapping->current_index * 60000); - - /* Initialize nulls and values arrays. */ - memset(nulls, 0, sizeof(nulls)); - values[0] = Int32GetDatum(array_position); - values[1] = TimestampTzGetDatum(timestamp); - values[2] = TransactionIdGetDatum(mapping->xid_by_minute[array_position]); - - return heap_form_tuple(tupdesc, values, nulls); -} diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 694d667bf97..f0a50a5f9ad 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2709,65 +2709,6 @@ include_dir 'conf.d' - - - old_snapshot_threshold (integer) - - old_snapshot_threshold configuration parameter - - - - - Sets the minimum amount of time that a query snapshot can be used - without risk of a snapshot too old error occurring - when using the snapshot. Data that has been dead for longer than - this threshold is allowed to be vacuumed away. This can help - prevent bloat in the face of snapshots which remain in use for a - long time. To prevent incorrect results due to cleanup of data which - would otherwise be visible to the snapshot, an error is generated - when the snapshot is older than this threshold and the snapshot is - used to read a page which has been modified since the snapshot was - built. - - - - If this value is specified without units, it is taken as minutes. - A value of -1 (the default) disables this feature, - effectively setting the snapshot age limit to infinity. - This parameter can only be set at server start. - - - - Useful values for production work probably range from a small number - of hours to a few days. Small values (such as 0 or - 1min) are only allowed because they may sometimes be - useful for testing. While a setting as high as 60d is - allowed, please note that in many workloads extreme bloat or - transaction ID wraparound may occur in much shorter time frames. - - - - When this feature is enabled, freed space at the end of a relation - cannot be released to the operating system, since that could remove - information needed to detect the snapshot too old - condition. All space allocated to a relation remains associated with - that relation for reuse only within that relation unless explicitly - freed (for example, with VACUUM FULL). - - - - This setting does not attempt to guarantee that an error will be - generated under any particular circumstances. In fact, if the - correct results can be generated from (for example) a cursor which - has materialized a result set, no error will be generated even if the - underlying rows in the referenced table have been vacuumed away. - Some tables cannot safely be vacuumed early, and so will not be - affected by this setting, such as system catalogs. For such tables - this setting will neither reduce bloat nor create a possibility - of a snapshot too old error on scanning. - - - @@ -4783,16 +4724,6 @@ ANY num_sync (