public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v24 05/15] Add Incremental View Maintenance support to pg_dump 3+ messages / 3 participants [nested] [flat]
* [PATCH v24 05/15] Add Incremental View Maintenance support to pg_dump @ 2020-11-11 08:01 Yugo Nagata <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Yugo Nagata @ 2020-11-11 08:01 UTC (permalink / raw) Support CREATE INCREMENTAL MATERIALIZED VIEW syntax. --- src/bin/pg_dump/pg_dump.c | 20 +++++++++++++++++--- src/bin/pg_dump/pg_dump.h | 1 + src/bin/pg_dump/t/002_pg_dump.pl | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index d1842edde0..d44361a47b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -6393,6 +6393,7 @@ getTables(Archive *fout, int *numTables) int i_partkeydef; int i_ispartition; int i_partbound; + int i_isivm; /* * Find all the tables and table-like objects. @@ -6596,12 +6597,19 @@ getTables(Archive *fout, int *numTables) appendPQExpBufferStr(query, "pg_get_partkeydef(c.oid) AS partkeydef, " "c.relispartition AS ispartition, " - "pg_get_expr(c.relpartbound, c.oid) AS partbound "); + "pg_get_expr(c.relpartbound, c.oid) AS partbound, "); else appendPQExpBufferStr(query, "NULL AS partkeydef, " "false AS ispartition, " - "NULL AS partbound "); + "NULL AS partbound, "); + + if (fout->remoteVersion >= 150000) + appendPQExpBufferStr(query, + "c.relisivm AS isivm "); + else + appendPQExpBufferStr(query, + "false AS isivm "); /* * Left join to pg_depend to pick up dependency info linking sequences to @@ -6719,6 +6727,8 @@ getTables(Archive *fout, int *numTables) i_partkeydef = PQfnumber(res, "partkeydef"); i_ispartition = PQfnumber(res, "ispartition"); i_partbound = PQfnumber(res, "partbound"); + i_isivm = PQfnumber(res, "isivm"); + if (dopt->lockWaitTimeout) { @@ -6796,6 +6806,8 @@ getTables(Archive *fout, int *numTables) tblinfo[i].partkeydef = pg_strdup(PQgetvalue(res, i, i_partkeydef)); tblinfo[i].ispartition = (strcmp(PQgetvalue(res, i, i_ispartition), "t") == 0); tblinfo[i].partbound = pg_strdup(PQgetvalue(res, i, i_partbound)); + tblinfo[i].isivm = (strcmp(PQgetvalue(res, i, i_isivm), "t") == 0); + /* other fields were zeroed above */ @@ -15868,9 +15880,11 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) binary_upgrade_set_pg_class_oids(fout, q, tbinfo->dobj.catId.oid, false); - appendPQExpBuffer(q, "CREATE %s%s %s", + appendPQExpBuffer(q, "CREATE %s%s%s %s", tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ? "UNLOGGED " : "", + tbinfo->relkind == RELKIND_MATVIEW && tbinfo->isivm ? + "INCREMENTAL " : "", reltypename, qualrelname); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index f9af14b793..c4e226831c 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -301,6 +301,7 @@ typedef struct _tableInfo bool dummy_view; /* view's real definition must be postponed */ bool postponed_def; /* matview must be postponed into post-data */ bool ispartition; /* is table a partition? */ + bool isivm; /* is incrementally maintainable materialized view? */ /* * These fields are computed only if we decide the table is interesting diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index d293f52b05..80efbcc7bc 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2180,6 +2180,21 @@ my %tests = ( { exclude_dump_test_schema => 1, no_toast_compression => 1, }, }, + 'CREATE MATERIALIZED VIEW matview_ivm' => { + create_order => 21, + create_sql => 'CREATE INCREMENTAL MATERIALIZED VIEW dump_test.matview_ivm (col1) AS + SELECT col1 FROM dump_test.test_table;', + regexp => qr/^ + \QCREATE INCREMENTAL MATERIALIZED VIEW dump_test.matview_ivm AS\E + \n\s+\QSELECT test_table.col1\E + \n\s+\QFROM dump_test.test_table\E + \n\s+\QWITH NO DATA;\E + /xm, + like => + { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, + unlike => { exclude_dump_test_schema => 1, }, + }, + 'CREATE POLICY p1 ON test_table' => { create_order => 22, create_sql => 'CREATE POLICY p1 ON dump_test.test_table -- 2.17.1 --Multipart=_Fri__29_Oct_2021_18_16_28_+0900_jlYRKjywLqhZ7oyk Content-Type: text/x-diff; name="v24-0006-Add-Incremental-View-Maintenance-support-to-psql.patch" Content-Disposition: attachment; filename="v24-0006-Add-Incremental-View-Maintenance-support-to-psql.patch" Content-Transfer-Encoding: 7bit ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Cross-database SERIALIZABLE safe snapshots @ 2023-07-10 09:17 Heikki Linnakangas <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Heikki Linnakangas @ 2023-07-10 09:17 UTC (permalink / raw) To: Thomas Munro <[email protected]>; pgsql-hackers On 09/03/2023 07:34, Thomas Munro wrote: > Here is a feature idea that emerged from a pgsql-bugs thread[1] that I > am kicking into the next commitfest. Example: > > s1: \c db1 > s1: CREATE TABLE t (i int); > s1: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; > s1: INSERT INTO t VALUES (42); > > s2: \c db2 > s2: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY DEFERRABLE; > s2: SELECT * FROM x; > > I don't know of any reason why s2 should have to wait, or > alternatively, without DEFERRABLE, why it shouldn't immediately drop > from SSI to SI (that is, opt out of predicate locking and go faster). > This change relies on the fact that PostgreSQL doesn't allow any kind > of cross-database access, except for shared catalogs, and all catalogs > are already exempt from SSI. I have updated this new version of the > patch to explain that very clearly at the place where that exemption > happens, so that future hackers would see that we rely on that fact > elsewhere if reconsidering that. Makes sense. > @@ -1814,7 +1823,17 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, > { > othersxact = dlist_container(SERIALIZABLEXACT, xactLink, iter.cur); > > - if (!SxactIsCommitted(othersxact) > + /* > + * We can't possibly have an unsafe conflict with a transaction in > + * another database. The only possible overlap is on shared > + * catalogs, but we don't support SSI for shared catalogs. The > + * invalid database case covers 2PC, because we don't yet record > + * database OIDs in the 2PC information. We also filter out doomed > + * transactions as they can't possibly commit. > + */ > + if ((othersxact->database == InvalidOid || > + othersxact->database == MyDatabaseId) > + && !SxactIsCommitted(othersxact) > && !SxactIsDoomed(othersxact) > && !SxactIsReadOnly(othersxact)) > { Why don't we set the database OID in 2PC transactions? We actually do set it correctly - or rather we never clear it - when a transaction is prepared. But you set it to invalid when recovering a prepared transaction on system startup. So the comment is a bit misleading: the optimization doesn't apply to 2PC transactions recovered after restart, other 2PC transactions are fine. I'm sure it's not a big deal in practice, but it's also not hard to fix. We do store the database OID in the twophase state. The caller of predicatelock_twophase_recover() has it, we just need a little plumbing to pass it down. Attached patches: 1. Rebased version of your patch, just trivial pgindent conflict fixes 2. Some comment typo fixes and improvements 3. Set the database ID on recovered 2PC transactions A test for this would be nice. isolationtester doesn't support connecting to different databases, restarting the server to test the 2PC recovery, but a TAP test could do it. -- Heikki Linnakangas Neon (https://neon.tech) Attachments: [text/x-patch] v2-0001-Optimize-cross-database-SERIALIZABLE-safe-snapsho.patch (5.1K, ../../[email protected]/2-v2-0001-Optimize-cross-database-SERIALIZABLE-safe-snapsho.patch) download | inline diff: From 7976215833c153d97c3d6de95bcfc17d2d530490 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Thu, 9 Mar 2023 18:12:17 +1300 Subject: [PATCH v2 1/3] Optimize cross-database SERIALIZABLE safe snapshots. SERIALIZABLE READ ONLY [DEFERRABLE] transactions can benefit from "safe snapshots", where they detect that no serializable anomalies could be created, so we can silently drop to the cheaper REPEATABLE READ (in other words from SSI to SI). Since a transactions connected to different databases can't access each others' data at all, except for catalogs where SSI doesn't apply anyway, there is no point in waiting for transactions in other databases. Filter them out while populating our possibleUnsafeConflicts lists. This means that non-DEFERRABLE safe snapshots might opt out immediately or sooner, and DEFERRABLE safe snapshots might not have to wait as long to get started, in scenarios where more than one database is using SERIALIZABLE transactions. Discussion: https://postgr.es/m/17116-d6ca217acc180e30%40postgresql.org --- src/backend/storage/lmgr/predicate.c | 28 +++++++++++++++++++---- src/include/storage/predicate_internals.h | 1 + 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 1af41213b4..5e62027284 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -489,6 +489,13 @@ static void ReleasePredicateLocksLocal(void); /* * Does this relation participate in predicate locking? Temporary and system * relations are exempt. + * + * Note that GetSerializableTransactionSnapshotInt() relies on the latter + * exemption when considering separate databases to be fully partitioned from + * each other for the purposes of the safe snapshot optimization. If SSI is + * ever applied to system catalogs, and in particular shared catalogs, + * databases would not no longer be sufficiently isolated and that would need + * to be reconsdered. */ static inline bool PredicateLockingNeededForRelation(Relation relation) @@ -1213,6 +1220,7 @@ InitPredicateLocks(void) PredXact->OldCommittedSxact->flags = SXACT_FLAG_COMMITTED; PredXact->OldCommittedSxact->pid = 0; PredXact->OldCommittedSxact->pgprocno = INVALID_PGPROCNO; + PredXact->OldCommittedSxact->database = InvalidOid; } /* This never changes, so let's keep a local copy. */ OldCommittedSxact = PredXact->OldCommittedSxact; @@ -1795,6 +1803,7 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, sxact->xmin = snapshot->xmin; sxact->pid = MyProcPid; sxact->pgprocno = MyProc->pgprocno; + sxact->database = MyDatabaseId; dlist_init(&sxact->predicateLocks); dlist_node_init(&sxact->finishedLink); sxact->flags = 0; @@ -1814,7 +1823,17 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, { othersxact = dlist_container(SERIALIZABLEXACT, xactLink, iter.cur); - if (!SxactIsCommitted(othersxact) + /* + * We can't possibly have an unsafe conflict with a transaction in + * another database. The only possible overlap is on shared + * catalogs, but we don't support SSI for shared catalogs. The + * invalid database case covers 2PC, because we don't yet record + * database OIDs in the 2PC information. We also filter out doomed + * transactions as they can't possibly commit. + */ + if ((othersxact->database == InvalidOid || + othersxact->database == MyDatabaseId) + && !SxactIsCommitted(othersxact) && !SxactIsDoomed(othersxact) && !SxactIsReadOnly(othersxact)) { @@ -1824,9 +1843,9 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, /* * If we didn't find any possibly unsafe conflicts because every - * uncommitted writable transaction turned out to be doomed, then we - * can "opt out" immediately. See comments above the earlier check - * for PredXact->WritableSxactCount == 0. + * uncommitted writable transaction turned out to be doomed or in + * another database, then we can "opt out" immediately. See comments + * above the earlier check for PredXact->WritableSxactCount == 0. */ if (dlist_is_empty(&sxact->possibleUnsafeConflicts)) { @@ -4875,6 +4894,7 @@ predicatelock_twophase_recover(TransactionId xid, uint16 info, sxact->vxid.localTransactionId = (LocalTransactionId) xid; sxact->pid = 0; sxact->pgprocno = INVALID_PGPROCNO; + sxact->database = InvalidOid; /* a prepared xact hasn't committed yet */ sxact->prepareSeqNo = RecoverySerCommitSeqNo; diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h index 93f84500bf..896026abdd 100644 --- a/src/include/storage/predicate_internals.h +++ b/src/include/storage/predicate_internals.h @@ -116,6 +116,7 @@ typedef struct SERIALIZABLEXACT uint32 flags; /* OR'd combination of values defined below */ int pid; /* pid of associated process */ int pgprocno; /* pgprocno of associated process */ + Oid database; /* which database is this transaction in? */ } SERIALIZABLEXACT; #define SXACT_FLAG_COMMITTED 0x00000001 /* already committed */ -- 2.30.2 [text/x-patch] v2-0002-Fix-typos-and-reword-comment.patch (1.6K, ../../[email protected]/3-v2-0002-Fix-typos-and-reword-comment.patch) download | inline diff: From f1c141cffe751a36a01a91f9c1a022b863f52dad Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas <[email protected]> Date: Mon, 10 Jul 2023 11:45:27 +0300 Subject: [PATCH v2 2/3] Fix typos and reword comment. --- src/backend/storage/lmgr/predicate.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 5e62027284..fa05c801b4 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -490,12 +490,12 @@ static void ReleasePredicateLocksLocal(void); * Does this relation participate in predicate locking? Temporary and system * relations are exempt. * - * Note that GetSerializableTransactionSnapshotInt() relies on the latter - * exemption when considering separate databases to be fully partitioned from - * each other for the purposes of the safe snapshot optimization. If SSI is - * ever applied to system catalogs, and in particular shared catalogs, - * databases would not no longer be sufficiently isolated and that would need - * to be reconsdered. + * Note that GetSerializableTransactionSnapshotInt() relies on system + * relations to be exempt. It assumes separate databases to be fully + * partitioned from each other for the purposes of the safe snapshot + * optimization. If SSI is ever applied to system catalogs, and in + * particular to shared system catalogs, databases would no longer be + * sufficiently isolated and that would need to be reconsidered. */ static inline bool PredicateLockingNeededForRelation(Relation relation) -- 2.30.2 [text/x-patch] v2-0003-Set-databaseid-when-recovering-an-prepared-SSI-tr.patch (13.0K, ../../[email protected]/4-v2-0003-Set-databaseid-when-recovering-an-prepared-SSI-tr.patch) download | inline diff: From 015ddb48571dae1ae2316bf33785685053b63b5c Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas <[email protected]> Date: Mon, 10 Jul 2023 12:02:44 +0300 Subject: [PATCH v2 3/3] Set databaseid when recovering an prepared SSI transaction. --- src/backend/access/transam/multixact.c | 8 ++++---- src/backend/access/transam/twophase.c | 12 ++++++------ src/backend/storage/lmgr/lock.c | 10 +++++----- src/backend/storage/lmgr/predicate.c | 14 ++++++-------- src/backend/utils/activity/pgstat_relation.c | 4 ++-- src/include/access/multixact.h | 6 +++--- src/include/access/twophase_rmgr.h | 2 +- src/include/pgstat.h | 4 ++-- src/include/storage/lock.h | 8 ++++---- src/include/storage/predicate.h | 2 +- 10 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index abb022e067..ac81b74b1f 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -1776,7 +1776,7 @@ PostPrepare_MultiXact(TransactionId xid) * Recover the state of a prepared transaction at startup */ void -multixact_twophase_recover(TransactionId xid, uint16 info, +multixact_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid, false); @@ -1797,7 +1797,7 @@ multixact_twophase_recover(TransactionId xid, uint16 info, * Similar to AtEOXact_MultiXact but for COMMIT PREPARED */ void -multixact_twophase_postcommit(TransactionId xid, uint16 info, +multixact_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { BackendId dummyBackendId = TwoPhaseGetDummyBackendId(xid, true); @@ -1812,10 +1812,10 @@ multixact_twophase_postcommit(TransactionId xid, uint16 info, * This is actually just the same as the COMMIT case. */ void -multixact_twophase_postabort(TransactionId xid, uint16 info, +multixact_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { - multixact_twophase_postcommit(xid, info, recdata, len); + multixact_twophase_postcommit(databaseid, xid, info, recdata, len); } /* diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 068e59bec0..fb9e4d16fa 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -219,7 +219,7 @@ static void RecordTransactionAbortPrepared(TransactionId xid, int nstats, xl_xact_stats_item *stats, const char *gid); -static void ProcessRecords(char *bufptr, TransactionId xid, +static void ProcessRecords(char *bufptr, Oid databaseid, TransactionId xid, const TwoPhaseCallback callbacks[]); static void RemoveGXact(GlobalTransaction gxact); @@ -1628,9 +1628,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit) /* And now do the callbacks */ if (isCommit) - ProcessRecords(bufptr, xid, twophase_postcommit_callbacks); + ProcessRecords(bufptr, hdr->database, xid, twophase_postcommit_callbacks); else - ProcessRecords(bufptr, xid, twophase_postabort_callbacks); + ProcessRecords(bufptr, hdr->database, xid, twophase_postabort_callbacks); PredicateLockTwoPhaseFinish(xid, isCommit); @@ -1663,7 +1663,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit) * Scan 2PC state data in memory and call the indicated callbacks for each 2PC record. */ static void -ProcessRecords(char *bufptr, TransactionId xid, +ProcessRecords(char *bufptr, Oid databaseid, TransactionId xid, const TwoPhaseCallback callbacks[]) { for (;;) @@ -1677,7 +1677,7 @@ ProcessRecords(char *bufptr, TransactionId xid, bufptr += MAXALIGN(sizeof(TwoPhaseRecordOnDisk)); if (callbacks[record->rmid] != NULL) - callbacks[record->rmid] (xid, record->info, + callbacks[record->rmid] (databaseid, xid, record->info, (void *) bufptr, record->len); bufptr += MAXALIGN(record->len); @@ -2123,7 +2123,7 @@ RecoverPreparedTransactions(void) /* * Recover other state (notably locks) using resource managers. */ - ProcessRecords(bufptr, xid, twophase_recover_callbacks); + ProcessRecords(bufptr, hdr->database, xid, twophase_recover_callbacks); /* * Release locks held by the standby process after we process each diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index f595bce31b..89f74ebee1 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -4122,7 +4122,7 @@ DumpAllLocks(void) * and PANIC anyway. */ void -lock_twophase_recover(TransactionId xid, uint16 info, +lock_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhaseLockRecord *rec = (TwoPhaseLockRecord *) recdata; @@ -4303,7 +4303,7 @@ lock_twophase_recover(TransactionId xid, uint16 info, * starting up into hot standby mode. */ void -lock_twophase_standby_recover(TransactionId xid, uint16 info, +lock_twophase_standby_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhaseLockRecord *rec = (TwoPhaseLockRecord *) recdata; @@ -4335,7 +4335,7 @@ lock_twophase_standby_recover(TransactionId xid, uint16 info, * Find and release the lock indicated by the 2PC record. */ void -lock_twophase_postcommit(TransactionId xid, uint16 info, +lock_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhaseLockRecord *rec = (TwoPhaseLockRecord *) recdata; @@ -4361,10 +4361,10 @@ lock_twophase_postcommit(TransactionId xid, uint16 info, * This is actually just the same as the COMMIT case. */ void -lock_twophase_postabort(TransactionId xid, uint16 info, +lock_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { - lock_twophase_postcommit(xid, info, recdata, len); + lock_twophase_postcommit(databaseid, xid, info, recdata, len); } /* diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index fa05c801b4..edfa0e05fa 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -1826,13 +1826,11 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, /* * We can't possibly have an unsafe conflict with a transaction in * another database. The only possible overlap is on shared - * catalogs, but we don't support SSI for shared catalogs. The - * invalid database case covers 2PC, because we don't yet record - * database OIDs in the 2PC information. We also filter out doomed - * transactions as they can't possibly commit. + * catalogs, but we don't support SSI for shared catalogs. We + * also filter out doomed transactions as they can't possibly + * commit. */ - if ((othersxact->database == InvalidOid || - othersxact->database == MyDatabaseId) + if (othersxact->database == MyDatabaseId && !SxactIsCommitted(othersxact) && !SxactIsDoomed(othersxact) && !SxactIsReadOnly(othersxact)) @@ -4859,7 +4857,7 @@ PredicateLockTwoPhaseFinish(TransactionId xid, bool isCommit) * Re-acquire a predicate lock belonging to a transaction that was prepared. */ void -predicatelock_twophase_recover(TransactionId xid, uint16 info, +predicatelock_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhasePredicateRecord *record; @@ -4894,7 +4892,7 @@ predicatelock_twophase_recover(TransactionId xid, uint16 info, sxact->vxid.localTransactionId = (LocalTransactionId) xid; sxact->pid = 0; sxact->pgprocno = INVALID_PGPROCNO; - sxact->database = InvalidOid; + sxact->database = databaseid; /* a prepared xact hasn't committed yet */ sxact->prepareSeqNo = RecoverySerCommitSeqNo; diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c index 9876e0c1e8..87670318ed 100644 --- a/src/backend/utils/activity/pgstat_relation.c +++ b/src/backend/utils/activity/pgstat_relation.c @@ -699,7 +699,7 @@ PostPrepare_PgStat_Relations(PgStat_SubXactStatus *xact_state) * Load the saved counts into our local pgstats state. */ void -pgstat_twophase_postcommit(TransactionId xid, uint16 info, +pgstat_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhasePgStatRecord *rec = (TwoPhasePgStatRecord *) recdata; @@ -735,7 +735,7 @@ pgstat_twophase_postcommit(TransactionId xid, uint16 info, * as aborted. */ void -pgstat_twophase_postabort(TransactionId xid, uint16 info, +pgstat_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len) { TwoPhasePgStatRecord *rec = (TwoPhasePgStatRecord *) recdata; diff --git a/src/include/access/multixact.h b/src/include/access/multixact.h index 246f757f6a..34646d15b5 100644 --- a/src/include/access/multixact.h +++ b/src/include/access/multixact.h @@ -149,11 +149,11 @@ extern void MultiXactAdvanceNextMXact(MultiXactId minMulti, extern void MultiXactAdvanceOldest(MultiXactId oldestMulti, Oid oldestMultiDB); extern int MultiXactMemberFreezeThreshold(void); -extern void multixact_twophase_recover(TransactionId xid, uint16 info, +extern void multixact_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void multixact_twophase_postcommit(TransactionId xid, uint16 info, +extern void multixact_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void multixact_twophase_postabort(TransactionId xid, uint16 info, +extern void multixact_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); extern void multixact_redo(XLogReaderState *record); diff --git a/src/include/access/twophase_rmgr.h b/src/include/access/twophase_rmgr.h index 02f7ba2872..751284b617 100644 --- a/src/include/access/twophase_rmgr.h +++ b/src/include/access/twophase_rmgr.h @@ -14,7 +14,7 @@ #ifndef TWOPHASE_RMGR_H #define TWOPHASE_RMGR_H -typedef void (*TwoPhaseCallback) (TransactionId xid, uint16 info, +typedef void (*TwoPhaseCallback) (Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); typedef uint8 TwoPhaseRmgrId; diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 57a2c0866a..f1a9fc8dcc 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -647,9 +647,9 @@ extern void pgstat_count_heap_delete(Relation rel); extern void pgstat_count_truncate(Relation rel); extern void pgstat_update_heap_dead_tuples(Relation rel, int delta); -extern void pgstat_twophase_postcommit(TransactionId xid, uint16 info, +extern void pgstat_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void pgstat_twophase_postabort(TransactionId xid, uint16 info, +extern void pgstat_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); extern PgStat_StatTabEntry *pgstat_fetch_stat_tabentry(Oid relid); diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 8575bea25c..953609b37e 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -591,13 +591,13 @@ extern BlockedProcsData *GetBlockerStatusData(int blocked_pid); extern xl_standby_lock *GetRunningTransactionLocks(int *nlocks); extern const char *GetLockmodeName(LOCKMETHODID lockmethodid, LOCKMODE mode); -extern void lock_twophase_recover(TransactionId xid, uint16 info, +extern void lock_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void lock_twophase_postcommit(TransactionId xid, uint16 info, +extern void lock_twophase_postcommit(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void lock_twophase_postabort(TransactionId xid, uint16 info, +extern void lock_twophase_postabort(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); -extern void lock_twophase_standby_recover(TransactionId xid, uint16 info, +extern void lock_twophase_standby_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); extern DeadLockState DeadLockCheck(PGPROC *proc); diff --git a/src/include/storage/predicate.h b/src/include/storage/predicate.h index cd48afa17b..6a420a035c 100644 --- a/src/include/storage/predicate.h +++ b/src/include/storage/predicate.h @@ -77,7 +77,7 @@ extern void PreCommit_CheckForSerializationFailure(void); extern void AtPrepare_PredicateLocks(void); extern void PostPrepare_PredicateLocks(TransactionId xid); extern void PredicateLockTwoPhaseFinish(TransactionId xid, bool isCommit); -extern void predicatelock_twophase_recover(TransactionId xid, uint16 info, +extern void predicatelock_twophase_recover(Oid databaseid, TransactionId xid, uint16 info, void *recdata, uint32 len); /* parallel query support */ -- 2.30.2 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Cross-database SERIALIZABLE safe snapshots @ 2024-01-21 02:05 vignesh C <[email protected]> parent: Heikki Linnakangas <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: vignesh C @ 2024-01-21 02:05 UTC (permalink / raw) To: Heikki Linnakangas <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers On Mon, 10 Jul 2023 at 14:48, Heikki Linnakangas <[email protected]> wrote: > > On 09/03/2023 07:34, Thomas Munro wrote: > > Here is a feature idea that emerged from a pgsql-bugs thread[1] that I > > am kicking into the next commitfest. Example: > > > > s1: \c db1 > > s1: CREATE TABLE t (i int); > > s1: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; > > s1: INSERT INTO t VALUES (42); > > > > s2: \c db2 > > s2: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE READ ONLY DEFERRABLE; > > s2: SELECT * FROM x; > > > > I don't know of any reason why s2 should have to wait, or > > alternatively, without DEFERRABLE, why it shouldn't immediately drop > > from SSI to SI (that is, opt out of predicate locking and go faster). > > This change relies on the fact that PostgreSQL doesn't allow any kind > > of cross-database access, except for shared catalogs, and all catalogs > > are already exempt from SSI. I have updated this new version of the > > patch to explain that very clearly at the place where that exemption > > happens, so that future hackers would see that we rely on that fact > > elsewhere if reconsidering that. > > Makes sense. > > > @@ -1814,7 +1823,17 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, > > { > > othersxact = dlist_container(SERIALIZABLEXACT, xactLink, iter.cur); > > > > - if (!SxactIsCommitted(othersxact) > > + /* > > + * We can't possibly have an unsafe conflict with a transaction in > > + * another database. The only possible overlap is on shared > > + * catalogs, but we don't support SSI for shared catalogs. The > > + * invalid database case covers 2PC, because we don't yet record > > + * database OIDs in the 2PC information. We also filter out doomed > > + * transactions as they can't possibly commit. > > + */ > > + if ((othersxact->database == InvalidOid || > > + othersxact->database == MyDatabaseId) > > + && !SxactIsCommitted(othersxact) > > && !SxactIsDoomed(othersxact) > > && !SxactIsReadOnly(othersxact)) > > { > > Why don't we set the database OID in 2PC transactions? We actually do > set it correctly - or rather we never clear it - when a transaction is > prepared. But you set it to invalid when recovering a prepared > transaction on system startup. So the comment is a bit misleading: the > optimization doesn't apply to 2PC transactions recovered after restart, > other 2PC transactions are fine. > > I'm sure it's not a big deal in practice, but it's also not hard to fix. > We do store the database OID in the twophase state. The caller of > predicatelock_twophase_recover() has it, we just need a little plumbing > to pass it down. > > Attached patches: > > 1. Rebased version of your patch, just trivial pgindent conflict fixes > 2. Some comment typo fixes and improvements > 3. Set the database ID on recovered 2PC transactions > > A test for this would be nice. isolationtester doesn't support > connecting to different databases, restarting the server to test the 2PC > recovery, but a TAP test could do it. @Thomas Munro As this patch is already marked as "Ready for Committer", do you want to take this patch forward based on Heikki's suggestions and get it committed? Regards, Vignesh ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-01-21 02:05 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-11-11 08:01 [PATCH v24 05/15] Add Incremental View Maintenance support to pg_dump Yugo Nagata <[email protected]> 2023-07-10 09:17 Re: Cross-database SERIALIZABLE safe snapshots Heikki Linnakangas <[email protected]> 2024-01-21 02:05 ` Re: Cross-database SERIALIZABLE safe snapshots vignesh C <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox