src/backend/access/index/genam.c | 5 +- src/backend/catalog/dependency.c | 88 +++++++++++++++++---- src/backend/catalog/pg_depend.c | 69 +++++++++++++++++ src/backend/catalog/pg_shdepend.c | 149 +++++++++++++++++++++++++++++++++++- src/backend/storage/ipc/procarray.c | 6 +- src/backend/utils/errcodes.txt | 1 + src/backend/utils/time/snapmgr.c | 58 ++++++++++++-- src/include/access/genam.h | 2 +- src/include/utils/snapmgr.h | 12 +++ 9 files changed, 365 insertions(+), 25 deletions(-) diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index 0aa26b448b..7459f37a21 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -558,7 +558,7 @@ systable_getnext(SysScanDesc sysscan) * good crosscheck that the caller is interested in the right tuple. */ bool -systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup) +systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup, bool dirtysnap) { Snapshot freshsnap; bool result; @@ -571,6 +571,9 @@ systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup) * acquire snapshots, so we need not register the snapshot. Those * facilities are too low-level to have any business scanning tables. */ + + UseDirtyCatalogSnapshot = dirtysnap; + freshsnap = GetCatalogSnapshot(RelationGetRelid(sysscan->heap_rel)); result = table_tuple_satisfies_snapshot(sysscan->heap_rel, diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 41093ea6ae..91d5383cbc 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -82,6 +82,7 @@ #include "utils/lsyscache.h" #include "utils/syscache.h" +#include "utils/snapmgr.h" /* * Deletion processing requires additional state for each ObjectAddress that @@ -104,6 +105,7 @@ typedef struct #define DEPFLAG_REVERSE 0x0040 /* reverse internal/extension link */ #define DEPFLAG_IS_PART 0x0080 /* has a partition dependency */ #define DEPFLAG_SUBOBJECT 0x0100 /* subobject of another deletable object */ +#define DEPFLAG_DIRTY 0x0200 /* reached thanks to dirty snapshot */ /* expansible list of ObjectAddresses */ @@ -567,6 +569,7 @@ findDependentObjects(const ObjectAddress *object, int maxDependentObjects; ObjectAddressStack mystack; ObjectAddressExtra extra; + Snapshot dirtySnapshot; /* * If the target object is already being visited in an outer recursion @@ -632,8 +635,17 @@ findDependentObjects(const ObjectAddress *object, nkeys = 2; } + /* + * We use a dirty snapshot so that we see all potential dependencies, + * committed or not. Without doing this we would miss objects created + * during in-flight transactions. + */ + UseDirtyCatalogSnapshot = true; + + dirtySnapshot = GetCatalogSnapshot(RelationGetRelid(*depRel)); + scan = systable_beginscan(*depRel, DependDependerIndexId, true, - NULL, nkeys, key); + dirtySnapshot, nkeys, key); /* initialize variables that loop may fill */ memset(&owningObject, 0, sizeof(owningObject)); @@ -647,6 +659,16 @@ findDependentObjects(const ObjectAddress *object, otherObject.objectId = foundDep->refobjid; otherObject.objectSubId = foundDep->refobjsubid; + /* + * dirtySnapshot->xmin is set to the tuple's xmin + * if that is another transaction that's still in + * progress; or to InvalidTransactionId if the + * tuple's xmin is committed good, committed dead, + * or my own xact. See snapshot.h comments. + */ + if(TransactionIdIsValid(dirtySnapshot->xmin)) + objflags |= DEPFLAG_DIRTY; + /* * When scanning dependencies of a whole object, we may find rows * linking sub-objects of the object to the object itself. (Normally, @@ -770,7 +792,7 @@ findDependentObjects(const ObjectAddress *object, * interesting anymore. We test this by checking the * pg_depend entry (see notes below). */ - if (!systable_recheck_tuple(scan, tup)) + if (!systable_recheck_tuple(scan, tup, true)) { systable_endscan(scan); ReleaseDeletionLock(&otherObject); @@ -934,8 +956,18 @@ findDependentObjects(const ObjectAddress *object, else nkeys = 2; + /* + * We use a dirty snapshot so that we see all potential dependencies, + * committed or not. Without doing this we would miss objects created + * during in-flight transactions. + */ + + UseDirtyCatalogSnapshot = true; + + dirtySnapshot = GetCatalogSnapshot(RelationGetRelid(*depRel)); + scan = systable_beginscan(*depRel, DependReferenceIndexId, true, - NULL, nkeys, key); + dirtySnapshot, nkeys, key); while (HeapTupleIsValid(tup = systable_getnext(scan))) { @@ -946,6 +978,10 @@ findDependentObjects(const ObjectAddress *object, otherObject.objectId = foundDep->objid; otherObject.objectSubId = foundDep->objsubid; + /* This tuple is for an in-flight transaction. */ + if(TransactionIdIsValid(dirtySnapshot->xmin)) + subflags |= DEPFLAG_DIRTY; + /* * If what we found is a sub-object of the current object, just ignore * it. (Normally, such a dependency is implicit, but we must make @@ -968,7 +1004,7 @@ findDependentObjects(const ObjectAddress *object, * if the pg_depend tuple we are looking at is still live. (If the * object got deleted, the tuple would have been deleted too.) */ - if (!systable_recheck_tuple(scan, tup)) + if (!systable_recheck_tuple(scan, tup, true)) { /* release the now-useless lock */ ReleaseDeletionLock(&otherObject); @@ -1112,6 +1148,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects, int numReportedClient = 0; int numNotReportedClient = 0; int i; + int num_dirty; /* * If we need to delete any partition-dependent objects, make sure that @@ -1123,10 +1160,16 @@ reportDependentObjects(const ObjectAddresses *targetObjects, * trigger this complaint is to explicitly try to delete one partition of * a partitioned object. */ + + num_dirty = 0; + for (i = 0; i < targetObjects->numrefs; i++) { const ObjectAddressExtra *extra = &targetObjects->extras[i]; + if (extra->flags & DEPFLAG_DIRTY) + num_dirty++; + if ((extra->flags & DEPFLAG_IS_PART) && !(extra->flags & DEPFLAG_PARTITION)) { @@ -1148,6 +1191,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects, * the work. */ if (behavior == DROP_CASCADE && + num_dirty == 0 && !message_level_is_interesting(msglevel)) return; @@ -1179,6 +1223,10 @@ reportDependentObjects(const ObjectAddresses *targetObjects, if (extra->flags & DEPFLAG_SUBOBJECT) continue; + /* We need a dirty snapshot to get its description. */ + if (extra->flags & DEPFLAG_DIRTY) + UseDirtyCatalogSnapshot = true; + objDesc = getObjectDescription(obj, false); /* @@ -1201,7 +1249,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects, (errmsg_internal("drop auto-cascades to %s", objDesc))); } - else if (behavior == DROP_RESTRICT) + else if (behavior == DROP_RESTRICT || (behavior == DROP_CASCADE && num_dirty > 0)) { char *otherDesc = getObjectDescription(&extra->dependee, false); @@ -1211,8 +1259,12 @@ reportDependentObjects(const ObjectAddresses *targetObjects, /* separate entries with a newline */ if (clientdetail.len != 0) appendStringInfoChar(&clientdetail, '\n'); - appendStringInfo(&clientdetail, _("%s depends on %s"), - objDesc, otherDesc); + if (!(extra->flags & DEPFLAG_DIRTY)) + appendStringInfo(&clientdetail, _("%s depends on %s"), + objDesc, otherDesc); + else + appendStringInfo(&clientdetail, _("%s (not yet committed) depends on %s"), + objDesc, otherDesc); numReportedClient++; } else @@ -1220,10 +1272,14 @@ reportDependentObjects(const ObjectAddresses *targetObjects, /* separate entries with a newline */ if (logdetail.len != 0) appendStringInfoChar(&logdetail, '\n'); - appendStringInfo(&logdetail, _("%s depends on %s"), - objDesc, otherDesc); - pfree(otherDesc); - ok = false; + if (!(extra->flags & DEPFLAG_DIRTY)) + appendStringInfo(&logdetail, _("%s depends on %s"), + objDesc, otherDesc); + else + appendStringInfo(&logdetail, _("%s (not yet committed) depends on %s"), + objDesc, otherDesc); + pfree(otherDesc); + ok = false; } else { @@ -1258,6 +1314,12 @@ reportDependentObjects(const ObjectAddresses *targetObjects, if (!ok) { + const char *hint_msg; + if (num_dirty > 0) + hint_msg = "DROP and DROP CASCADE won't work when there are uncommitted dependencies."; + else + hint_msg = "Use DROP ... CASCADE to drop the dependent objects too."; + if (origObject) ereport(ERROR, (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), @@ -1265,14 +1327,14 @@ reportDependentObjects(const ObjectAddresses *targetObjects, getObjectDescription(origObject, false)), errdetail("%s", clientdetail.data), errdetail_log("%s", logdetail.data), - errhint("Use DROP ... CASCADE to drop the dependent objects too."))); + errhint("%s", hint_msg))); else ereport(ERROR, (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), errmsg("cannot drop desired object(s) because other objects depend on them"), errdetail("%s", clientdetail.data), errdetail_log("%s", logdetail.data), - errhint("Use DROP ... CASCADE to drop the dependent objects too."))); + errhint("%s", hint_msg))); } else if (numReportedClient > 1) { diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 1217c01b8a..dab2b56abe 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -30,6 +30,7 @@ #include "utils/lsyscache.h" #include "utils/pg_locale.h" #include "utils/rel.h" +#include "utils/snapmgr.h" static bool isObjectPinned(const ObjectAddress *object, Relation rel); @@ -73,6 +74,12 @@ recordMultipleDependencies(const ObjectAddress *depender, max_slots, slot_init_count, slot_stored_count; + Relation catalog; + HeapTuple tuple; + Oid oidIndexId; + SysScanDesc scan; + ScanKeyData skey; + Snapshot dirtySnapshot; if (nreferenced <= 0) return; /* nothing to do */ @@ -84,6 +91,68 @@ recordMultipleDependencies(const ObjectAddress *depender, if (IsBootstrapProcessingMode()) return; + catalog = table_open(referenced->classId, AccessShareLock); + oidIndexId = get_object_oid_index(referenced->classId); + + Assert(OidIsValid(oidIndexId)); + + /* + * We use a dirty snapshot so that we see all potential dependencies, + * committed or not. Without doing this we would miss objects created + * during in-flight transactions. + */ + UseDirtyCatalogSnapshot = true; + + dirtySnapshot = GetCatalogSnapshot(RelationGetRelid(catalog)); + + ScanKeyInit(&skey, + get_object_attnum_oid(referenced->classId), + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(referenced->objectId)); + + scan = systable_beginscan(catalog, oidIndexId, true, + dirtySnapshot, 1, &skey); + + tuple = systable_getnext(scan); + + /* + * dirtySnapshot->xmax is set to the tuple's xmax + * if that is another transaction that's still in + * progress; or to InvalidTransactionId if the + * tuple's xmax is committed good, committed dead, + * or my own xact. See snapshot.h comments. + */ + if (HeapTupleIsValid(tuple) && TransactionIdIsValid(dirtySnapshot->xmax)) + { + char *dependerDesc; + char *referencedDesc; + StringInfoData detail; + + initStringInfo(&detail); + + /* We need a dirty snapshot to get its description. */ + UseDirtyCatalogSnapshot = true; + dependerDesc = getObjectDescription(depender, false); + + referencedDesc = getObjectDescription(referenced, false); + + + appendStringInfo(&detail, _("%s depends on %s (dependency not yet committed)"), + dependerDesc, + referencedDesc); + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_UNCOMMITTED_DEPENDENCY), + errmsg("cannot create %s because it depends of other objects uncommitted dependencies", + dependerDesc)), + errdetail("%s", detail.data), + errdetail_log("%s", detail.data), + errhint("%s", "CREATE won't work as long as there is uncommitted dependencies.")); + } + + systable_endscan(scan); + table_close(catalog, AccessShareLock); + dependDesc = table_open(DependRelationId, RowExclusiveLock); /* diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index 90b7a5de29..51c0d089e1 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -66,6 +66,7 @@ #include "utils/acl.h" #include "utils/fmgroids.h" #include "utils/syscache.h" +#include "utils/snapmgr.h" typedef enum { @@ -123,6 +124,12 @@ recordSharedDependencyOn(ObjectAddress *depender, SharedDependencyType deptype) { Relation sdepRel; + Relation catalog; + HeapTuple tuple; + Oid oidIndexId; + SysScanDesc scan; + ScanKeyData skey; + Snapshot dirtySnapshot; /* * Objects in pg_shdepend can't have SubIds. @@ -137,6 +144,67 @@ recordSharedDependencyOn(ObjectAddress *depender, if (IsBootstrapProcessingMode()) return; + catalog = table_open(referenced->classId, AccessShareLock); + oidIndexId = get_object_oid_index(referenced->classId); + + Assert(OidIsValid(oidIndexId)); + + /* + * We use a dirty snapshot so that we see all potential dependencies, + * committed or not. Without doing this we would miss objects created + * during in-flight transactions. + */ + UseDirtyCatalogSnapshot = true; + + dirtySnapshot = GetCatalogSnapshot(RelationGetRelid(catalog)); + + ScanKeyInit(&skey, + get_object_attnum_oid(referenced->classId), + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(referenced->objectId)); + + scan = systable_beginscan(catalog, oidIndexId, true, + dirtySnapshot, 1, &skey); + + tuple = systable_getnext(scan); + + /* + * dirtySnapshot->xmax is set to the tuple's xmax + * if that is another transaction that's still in + * progress; or to InvalidTransactionId if the + * tuple's xmax is committed good, committed dead, + * or my own xact. See snapshot.h comments. + */ + if (HeapTupleIsValid(tuple) && TransactionIdIsValid(dirtySnapshot->xmax)) + { + char *dependerDesc; + char *referencedDesc; + StringInfoData detail; + + initStringInfo(&detail); + + /* We need a dirty snapshot to get its description. */ + UseDirtyCatalogSnapshot = true; + referencedDesc = getObjectDescription(referenced, false); + + dependerDesc = getObjectDescription(depender, false); + + appendStringInfo(&detail, _("%s depends on %s (modifications not yet committed)"), + dependerDesc, + referencedDesc); + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_UNCOMMITTED_DEPENDENCY), + errmsg("cannot create %s because it depends of other objects uncommitted dependencies", + dependerDesc)), + errdetail("%s", detail.data), + errdetail_log("%s", detail.data), + errhint("%s", "CREATE won't work as long as there is uncommitted modification dependencies.")); + } + + systable_endscan(scan); + table_close(catalog, AccessShareLock); + sdepRel = table_open(SharedDependRelationId, RowExclusiveLock); /* If the referenced object is pinned, do nothing. */ @@ -1357,6 +1425,7 @@ shdepDropOwned(List *roleids, DropBehavior behavior) ScanKeyData key[2]; SysScanDesc scan; HeapTuple tuple; + Snapshot dirtySnapshot; /* Doesn't work for pinned objects */ if (isSharedObjectPinned(AuthIdRelationId, roleid, sdepRel)) @@ -1374,6 +1443,15 @@ shdepDropOwned(List *roleids, DropBehavior behavior) getObjectDescription(&obj, false)))); } + /* + * We use a dirty snapshot so that we see all potential dependencies, + * committed or not. Without doing this we would miss objects created + * during in-flight transactions. + */ + UseDirtyCatalogSnapshot = true; + + dirtySnapshot = GetCatalogSnapshot(RelationGetRelid(sdepRel)); + ScanKeyInit(&key[0], Anum_pg_shdepend_refclassid, BTEqualStrategyNumber, F_OIDEQ, @@ -1384,7 +1462,7 @@ shdepDropOwned(List *roleids, DropBehavior behavior) ObjectIdGetDatum(roleid)); scan = systable_beginscan(sdepRel, SharedDependReferenceIndexId, true, - NULL, 2, key); + dirtySnapshot, 2, key); while ((tuple = systable_getnext(scan)) != NULL) { @@ -1432,11 +1510,43 @@ shdepDropOwned(List *roleids, DropBehavior behavior) * object in that case. */ AcquireDeletionLock(&obj, 0); - if (!systable_recheck_tuple(scan, tuple)) + if (!systable_recheck_tuple(scan, tuple, true)) { ReleaseDeletionLock(&obj); break; } + + /* + * dirtySnapshot->xmin is set to the tuple's xmin + * if that is another transaction that's still in + * progress; or to InvalidTransactionId if the + * tuple's xmin is committed good, committed dead, + * or my own xact. See snapshot.h comments. + */ + if(TransactionIdIsValid(dirtySnapshot->xmin)) + { + ObjectAddress roleobj; + char *roledesc; + char *objdesc; + + roleobj.classId = AuthIdRelationId; + roleobj.objectId = roleid; + roleobj.objectSubId = 0; + + roledesc = getObjectDescription(&roleobj, false); + + /* We need a dirty snapshot to get its description. */ + UseDirtyCatalogSnapshot = true; + objdesc = getObjectDescription(&obj, false); + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), + errmsg("cannot drop objects owned by %s because other uncommitted objects depend on it", + roledesc), + errdetail("%s (not yet committed) depends on %s", objdesc, roledesc), + errdetail_log("%s (not yet committed) depends on %s", objdesc, roledesc), + errhint("Commit or rollback %s creation.", objdesc))); + } add_exact_object_address(&obj, deleteobjs); } break; @@ -1449,11 +1559,44 @@ shdepDropOwned(List *roleids, DropBehavior behavior) obj.objectSubId = sdepForm->objsubid; /* as above */ AcquireDeletionLock(&obj, 0); - if (!systable_recheck_tuple(scan, tuple)) + if (!systable_recheck_tuple(scan, tuple, true)) { ReleaseDeletionLock(&obj); break; } + + /* + * dirtySnapshot->xmin is set to the tuple's xmin + * if that is another transaction that's still in + * progress; or to InvalidTransactionId if the + * tuple's xmin is committed good, committed dead, + * or my own xact. See snapshot.h comments. + */ + if(TransactionIdIsValid(dirtySnapshot->xmin)) + { + + ObjectAddress roleobj; + char *roledesc; + char *objdesc; + + roleobj.classId = AuthIdRelationId; + roleobj.objectId = roleid; + roleobj.objectSubId = 0; + + roledesc = getObjectDescription(&roleobj, false); + + /* We need a dirty snapshot to get its description. */ + UseDirtyCatalogSnapshot = true; + objdesc = getObjectDescription(&obj, false); + + ereport(ERROR, + (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), + errmsg("cannot drop objects owned by %s because other uncommitted objects depend on it", + roledesc), + errdetail("%s (not yet committed) depends on %s", objdesc, roledesc), + errdetail_log("%s (not yet committed) depends on %s", objdesc, roledesc), + errhint("Commit or rollback %s creation.", objdesc))); + } add_exact_object_address(&obj, deleteobjs); } break; diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 5ff8cab394..282bf0fb8f 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2195,7 +2195,11 @@ GetSnapshotData(Snapshot snapshot) */ LWLockAcquire(ProcArrayLock, LW_SHARED); - if (GetSnapshotDataReuse(snapshot)) + /* + * A dirty snapshot is not a candidate for GetSnapshotDataReuse + * as its xmin and/or xmax may have changed + */ + if (!IsDirtySnapshot(snapshot) && GetSnapshotDataReuse(snapshot)) { LWLockRelease(ProcArrayLock); return snapshot; diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt index 9874a77805..842cd1e854 100644 --- a/src/backend/utils/errcodes.txt +++ b/src/backend/utils/errcodes.txt @@ -270,6 +270,7 @@ Section: Class 2B - Dependent Privilege Descriptors Still Exist 2B000 E ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST dependent_privilege_descriptors_still_exist 2BP01 E ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST dependent_objects_still_exist +2BP02 E ERRCODE_DEPENDENT_OBJECTS_UNCOMMITTED_DEPENDENCY dependent_objects_uncommitted_dependency Section: Class 2D - Invalid Transaction Termination diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 95704265b6..314257530e 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -95,6 +95,7 @@ volatile OldSnapshotControlData *oldSnapshotControl; static SnapshotData CurrentSnapshotData = {SNAPSHOT_MVCC}; static SnapshotData SecondarySnapshotData = {SNAPSHOT_MVCC}; SnapshotData CatalogSnapshotData = {SNAPSHOT_MVCC}; +SnapshotData DirtyCatalogSnapshotData = {SNAPSHOT_DIRTY}; SnapshotData SnapshotSelfData = {SNAPSHOT_SELF}; SnapshotData SnapshotAnyData = {SNAPSHOT_ANY}; @@ -102,6 +103,7 @@ SnapshotData SnapshotAnyData = {SNAPSHOT_ANY}; static Snapshot CurrentSnapshot = NULL; static Snapshot SecondarySnapshot = NULL; static Snapshot CatalogSnapshot = NULL; +static Snapshot DirtyCatalogSnapshot = NULL; static Snapshot HistoricSnapshot = NULL; /* @@ -147,6 +149,7 @@ static pairingheap RegisteredSnapshots = {&xmin_cmp, NULL, NULL}; /* first GetTransactionSnapshot call in a transaction? */ bool FirstSnapshotSet = false; +bool UseDirtyCatalogSnapshot = false; /* * Remember the serializable transaction snapshot, if any. We cannot trust @@ -310,6 +313,7 @@ GetTransactionSnapshot(void) /* Don't allow catalog snapshot to be older than xact snapshot. */ InvalidateCatalogSnapshot(); + InvalidateDirtyCatalogSnapshot(); CurrentSnapshot = GetSnapshotData(&CurrentSnapshotData); @@ -407,6 +411,9 @@ GetCatalogSnapshot(Oid relid) Snapshot GetNonHistoricCatalogSnapshot(Oid relid) { + Snapshot *snap; + SnapshotData *snapdata; + /* * If the caller is trying to scan a relation that has no syscache, no * catcache invalidations will be sent when it is updated. For a few key @@ -414,15 +421,31 @@ GetNonHistoricCatalogSnapshot(Oid relid) * scan a relation for which neither catcache nor snapshot invalidations * are sent, we must refresh the snapshot every time. */ - if (CatalogSnapshot && + if (!UseDirtyCatalogSnapshot) + { + snap = &CatalogSnapshot; + snapdata = &CatalogSnapshotData; + } + else + { + snap = &DirtyCatalogSnapshot; + snapdata = &DirtyCatalogSnapshotData; + } + + if (*snap && !RelationInvalidatesSnapshotsOnly(relid) && !RelationHasSysCache(relid)) - InvalidateCatalogSnapshot(); + { + if (!UseDirtyCatalogSnapshot) + InvalidateCatalogSnapshot(); + else + InvalidateDirtyCatalogSnapshot(); + } - if (CatalogSnapshot == NULL) + if (*snap == NULL) { /* Get new snapshot. */ - CatalogSnapshot = GetSnapshotData(&CatalogSnapshotData); + *snap = GetSnapshotData(snapdata); /* * Make sure the catalog snapshot will be accounted for in decisions @@ -436,10 +459,11 @@ GetNonHistoricCatalogSnapshot(Oid relid) * NB: it had better be impossible for this to throw error, since the * CatalogSnapshot pointer is already valid. */ - pairingheap_add(&RegisteredSnapshots, &CatalogSnapshot->ph_node); + pairingheap_add(&RegisteredSnapshots, &snapdata->ph_node); } - return CatalogSnapshot; + UseDirtyCatalogSnapshot = false; + return *snap; } /* @@ -463,6 +487,26 @@ InvalidateCatalogSnapshot(void) } } +/* + * InvalidateDirtyCatalogSnapshot + * Mark the current catalog snapshot, if any, as invalid + * + * We could change this API to allow the caller to provide more fine-grained + * invalidation details, so that a change to relation A wouldn't prevent us + * from using our cached snapshot to scan relation B, but so far there's no + * evidence that the CPU cycles we spent tracking such fine details would be + * well-spent. + */ +void +InvalidateDirtyCatalogSnapshot(void) +{ + if (DirtyCatalogSnapshot) + { + pairingheap_remove(&RegisteredSnapshots, &DirtyCatalogSnapshot->ph_node); + DirtyCatalogSnapshot = NULL; + SnapshotResetXmin(); + } +} /* * InvalidateCatalogSnapshotConditionally * Drop catalog snapshot if it's the only one we have @@ -1057,6 +1101,7 @@ AtEOXact_Snapshot(bool isCommit, bool resetXmin) /* Drop catalog snapshot if any */ InvalidateCatalogSnapshot(); + InvalidateDirtyCatalogSnapshot(); /* On commit, complain about leftover snapshots */ if (isCommit) @@ -1083,6 +1128,7 @@ AtEOXact_Snapshot(bool isCommit, bool resetXmin) SecondarySnapshot = NULL; FirstSnapshotSet = false; + UseDirtyCatalogSnapshot = false; /* * During normal commit processing, we call ProcArrayEndTransaction() to diff --git a/src/include/access/genam.h b/src/include/access/genam.h index 480a4762f5..0c0db3fb74 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -218,7 +218,7 @@ extern SysScanDesc systable_beginscan(Relation heapRelation, Snapshot snapshot, int nkeys, ScanKey key); extern HeapTuple systable_getnext(SysScanDesc sysscan); -extern bool systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup); +extern bool systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup, bool dirtysnap); extern void systable_endscan(SysScanDesc sysscan); extern SysScanDesc systable_beginscan_ordered(Relation heapRelation, Relation indexRelation, diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index 44539fe15a..ea6687a562 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -54,6 +54,12 @@ extern TimestampTz GetOldSnapshotThresholdTimestamp(void); extern void SnapshotTooOldMagicForTest(void); extern bool FirstSnapshotSet; +/* + * Used to cover cases where a dirty + * catalog snapshot is needed. Currently + * used to avoid orphaned dependencies. + */ +extern bool UseDirtyCatalogSnapshot; extern PGDLLIMPORT TransactionId TransactionXmin; extern PGDLLIMPORT TransactionId RecentXmin; @@ -62,6 +68,7 @@ extern PGDLLIMPORT TransactionId RecentXmin; extern PGDLLIMPORT SnapshotData SnapshotSelfData; extern PGDLLIMPORT SnapshotData SnapshotAnyData; extern PGDLLIMPORT SnapshotData CatalogSnapshotData; +extern PGDLLIMPORT SnapshotData DirtyCatalogSnapshotData; #define SnapshotSelf (&SnapshotSelfData) #define SnapshotAny (&SnapshotAnyData) @@ -97,6 +104,10 @@ extern PGDLLIMPORT SnapshotData CatalogSnapshotData; ((snapshot)->snapshot_type == SNAPSHOT_MVCC || \ (snapshot)->snapshot_type == SNAPSHOT_HISTORIC_MVCC) +/* This macro encodes the knowledge of which snapshots are Dirty */ +#define IsDirtySnapshot(snapshot) \ + ((snapshot)->snapshot_type == SNAPSHOT_DIRTY) + static inline bool OldSnapshotThresholdActive(void) { @@ -111,6 +122,7 @@ extern Snapshot GetOldestSnapshot(void); extern Snapshot GetCatalogSnapshot(Oid relid); extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid); extern void InvalidateCatalogSnapshot(void); +extern void InvalidateDirtyCatalogSnapshot(void); extern void InvalidateCatalogSnapshotConditionally(void); extern void PushActiveSnapshot(Snapshot snapshot);