From ffaa44cb559db332baeee7d25dedd74a61974203 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 15 Nov 2016 15:55:35 -0500 Subject: Account for catalog snapshot in PGXACT->xmin updates. The CatalogSnapshot was not plugged into SnapshotResetXmin()'s accounting for whether MyPgXact->xmin could be cleared or advanced. In normal transactions this was masked by the fact that the transaction snapshot would be older, but during backend startup and certain utility commands it was possible to re-use the CatalogSnapshot after MyPgXact->xmin had been cleared, meaning that recently-deleted rows could be pruned even though this snapshot could still see them, causing unexpected catalog lookup failures. This effect appears to be the explanation for a recent failure on buildfarm member piculet. To fix, add the CatalogSnapshot to the RegisteredSnapshots heap whenever it is valid. In the previous logic, it was possible for the CatalogSnapshot to remain valid across waits for client input, but with this change that would mean it delays advance of global xmin in cases where it did not before. To avoid possibly causing new table-bloat problems with clients that sit idle for long intervals, add code to invalidate the CatalogSnapshot before waiting for client input. (When the backend is busy, it's unlikely that the CatalogSnapshot would be the oldest snap for very long, so we don't worry about forcing early invalidation of it otherwise.) In passing, remove the CatalogSnapshotStale flag in favor of using "CatalogSnapshot != NULL" to represent validity, as we do for the other special snapshots in snapmgr.c. And improve some obsolete comments. No regression test because I don't know a deterministic way to cause this failure. But the stress test shown in the original discussion provokes "cache lookup failed for relation 1255" within a few dozen seconds for me. Back-patch to 9.4 where MVCC catalog scans were introduced. (Note: it's quite easy to produce similar failures with the same test case in branches before 9.4. But MVCC catalog scans were supposed to fix that.) Discussion: <16447.1478818294@sss.pgh.pa.us> --- src/backend/tcop/postgres.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/backend/tcop/postgres.c') diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 599874e7435..3d9319d151e 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3946,6 +3946,12 @@ PostgresMain(int argc, char *argv[], initStringInfo(&input_message); + /* + * Also consider releasing our catalog snapshot if any, so that it's + * not preventing advance of global xmin while we wait for the client. + */ + InvalidateCatalogSnapshotConditionally(); + /* * (1) If we've reached idle state, tell the frontend we're ready for * a new query. @@ -4422,7 +4428,7 @@ ShowUsage(const char *title) appendStringInfoString(&str, "! system usage stats:\n"); appendStringInfo(&str, - "!\t%ld.%06ld s user, %ld.%06ld s system, %ld.%06ld s elapsed\n", + "!\t%ld.%06ld s user, %ld.%06ld s system, %ld.%06ld s elapsed\n", (long) (r.ru_utime.tv_sec - Save_r.ru_utime.tv_sec), (long) (r.ru_utime.tv_usec - Save_r.ru_utime.tv_usec), (long) (r.ru_stime.tv_sec - Save_r.ru_stime.tv_sec), -- cgit v1.2.3