Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit568d413

Browse files
committed
Use an MVCC snapshot, rather than SnapshotNow, for catalog scans.
SnapshotNow scans have the undesirable property that, in the face ofconcurrent updates, the scan can fail to see either the old or the newversions of the row. In many cases, we work around this by requiringDDL operations to hold AccessExclusiveLock on the object beingmodified; in some cases, the existing locking is inadequate and randomfailures occur as a result. This commit doesn't change anythingrelated to locking, but will hopefully pave the way to allowing lockstrength reductions in the future.The major issue has held us back from making this change in the pastis that taking an MVCC snapshot is significantly more expensive thanusing a static special snapshot such as SnapshotNow. However, testingof various worst-case scenarios reveals that this problem is notsevere except under fairly extreme workloads. To mitigate thoseproblems, we avoid retaking the MVCC snapshot for each new scan;instead, we take a new snapshot only when invalidation messages havebeen processed. The catcache machinery already requires thatinvalidation messages be sent before releasing the related heavyweightlock; else other backends might rely on locally-cached data ratherthan scanning the catalog at all. Thus, making snapshot reusedependent on the same guarantees shouldn't break anything that wasn'talready subtly broken.Patch by me. Review by Michael Paquier and Andres Freund.
1 parent384f933 commit568d413

File tree

69 files changed

+616
-352
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+616
-352
lines changed

‎contrib/dblink/dblink.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2046,7 +2046,7 @@ get_pkey_attnames(Relation rel, int16 *numatts)
20462046
ObjectIdGetDatum(RelationGetRelid(rel)));
20472047

20482048
scan=systable_beginscan(indexRelation,IndexIndrelidIndexId, true,
2049-
SnapshotNow,1,&skey);
2049+
NULL,1,&skey);
20502050

20512051
while (HeapTupleIsValid(indexTuple=systable_getnext(scan)))
20522052
{

‎contrib/sepgsql/label.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
727727
rel=heap_open(catalogId,AccessShareLock);
728728

729729
sscan=systable_beginscan(rel,InvalidOid, false,
730-
SnapshotNow,0,NULL);
730+
NULL,0,NULL);
731731
while (HeapTupleIsValid(tuple=systable_getnext(sscan)))
732732
{
733733
Form_pg_databasedatForm;

‎doc/src/sgml/indexam.sgml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ amrestrpos (IndexScanDesc scan);
713713
When using an MVCC-compliant snapshot, there is no problem because
714714
the new occupant of the slot is certain to be too new to pass the
715715
snapshot test. However, with a non-MVCC-compliant snapshot (such as
716-
<literal>SnapshotNow</>), it would be possible to accept and return
716+
<literal>SnapshotAny</>), it would be possible to accept and return
717717
a row that does not in fact match the scan keys. We could defend
718718
against this scenario by requiring the scan keys to be rechecked
719719
against the heap row in all cases, but that is too expensive. Instead,

‎src/backend/access/heap/heapam.c‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ static HeapScanDesc heap_beginscan_internal(Relation relation,
8080
Snapshotsnapshot,
8181
intnkeys,ScanKeykey,
8282
boolallow_strat,boolallow_sync,
83-
boolis_bitmapscan);
83+
boolis_bitmapscan,booltemp_snap);
8484
staticHeapTupleheap_prepare_insert(Relationrelation,HeapTupletup,
8585
TransactionIdxid,CommandIdcid,intoptions);
8686
staticXLogRecPtrlog_heap_update(Relationreln,Bufferoldbuf,
@@ -1286,7 +1286,17 @@ heap_beginscan(Relation relation, Snapshot snapshot,
12861286
intnkeys,ScanKeykey)
12871287
{
12881288
returnheap_beginscan_internal(relation,snapshot,nkeys,key,
1289-
true, true, false);
1289+
true, true, false, false);
1290+
}
1291+
1292+
HeapScanDesc
1293+
heap_beginscan_catalog(Relationrelation,intnkeys,ScanKeykey)
1294+
{
1295+
Oidrelid=RelationGetRelid(relation);
1296+
Snapshotsnapshot=RegisterSnapshot(GetCatalogSnapshot(relid));
1297+
1298+
returnheap_beginscan_internal(relation,snapshot,nkeys,key,
1299+
true, true, false, true);
12901300
}
12911301

12921302
HeapScanDesc
@@ -1295,22 +1305,22 @@ heap_beginscan_strat(Relation relation, Snapshot snapshot,
12951305
boolallow_strat,boolallow_sync)
12961306
{
12971307
returnheap_beginscan_internal(relation,snapshot,nkeys,key,
1298-
allow_strat,allow_sync, false);
1308+
allow_strat,allow_sync, false, false);
12991309
}
13001310

13011311
HeapScanDesc
13021312
heap_beginscan_bm(Relationrelation,Snapshotsnapshot,
13031313
intnkeys,ScanKeykey)
13041314
{
13051315
returnheap_beginscan_internal(relation,snapshot,nkeys,key,
1306-
false, false, true);
1316+
false, false, true, false);
13071317
}
13081318

13091319
staticHeapScanDesc
13101320
heap_beginscan_internal(Relationrelation,Snapshotsnapshot,
13111321
intnkeys,ScanKeykey,
13121322
boolallow_strat,boolallow_sync,
1313-
boolis_bitmapscan)
1323+
boolis_bitmapscan,booltemp_snap)
13141324
{
13151325
HeapScanDescscan;
13161326

@@ -1335,6 +1345,7 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot,
13351345
scan->rs_strategy=NULL;/* set in initscan */
13361346
scan->rs_allow_strat=allow_strat;
13371347
scan->rs_allow_sync=allow_sync;
1348+
scan->rs_temp_snap=temp_snap;
13381349

13391350
/*
13401351
* we can use page-at-a-time mode if it's an MVCC-safe snapshot
@@ -1421,6 +1432,9 @@ heap_endscan(HeapScanDesc scan)
14211432
if (scan->rs_strategy!=NULL)
14221433
FreeAccessStrategy(scan->rs_strategy);
14231434

1435+
if (scan->rs_temp_snap)
1436+
UnregisterSnapshot(scan->rs_snapshot);
1437+
14241438
pfree(scan);
14251439
}
14261440

‎src/backend/access/index/genam.c‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include"utils/builtins.h"
2929
#include"utils/lsyscache.h"
3030
#include"utils/rel.h"
31+
#include"utils/snapmgr.h"
3132
#include"utils/tqual.h"
3233

3334

@@ -231,7 +232,7 @@ BuildIndexValueDescription(Relation indexRelation,
231232
*rel: catalog to scan, already opened and suitably locked
232233
*indexId: OID of index to conditionally use
233234
*indexOK: if false, forces a heap scan (see notes below)
234-
*snapshot: time qual to use (usually should be SnapshotNow)
235+
*snapshot: time qual to use (NULL for a recent catalog snapshot)
235236
*nkeys, key: scan keys
236237
*
237238
* The attribute numbers in the scan key should be set for the heap case.
@@ -266,6 +267,19 @@ systable_beginscan(Relation heapRelation,
266267
sysscan->heap_rel=heapRelation;
267268
sysscan->irel=irel;
268269

270+
if (snapshot==NULL)
271+
{
272+
Oidrelid=RelationGetRelid(heapRelation);
273+
274+
snapshot=RegisterSnapshot(GetCatalogSnapshot(relid));
275+
sysscan->snapshot=snapshot;
276+
}
277+
else
278+
{
279+
/* Caller is responsible for any snapshot. */
280+
sysscan->snapshot=NULL;
281+
}
282+
269283
if (irel)
270284
{
271285
inti;
@@ -401,6 +415,9 @@ systable_endscan(SysScanDesc sysscan)
401415
else
402416
heap_endscan(sysscan->scan);
403417

418+
if (sysscan->snapshot)
419+
UnregisterSnapshot(sysscan->snapshot);
420+
404421
pfree(sysscan);
405422
}
406423

@@ -444,6 +461,19 @@ systable_beginscan_ordered(Relation heapRelation,
444461
sysscan->heap_rel=heapRelation;
445462
sysscan->irel=indexRelation;
446463

464+
if (snapshot==NULL)
465+
{
466+
Oidrelid=RelationGetRelid(heapRelation);
467+
468+
snapshot=RegisterSnapshot(GetCatalogSnapshot(relid));
469+
sysscan->snapshot=snapshot;
470+
}
471+
else
472+
{
473+
/* Caller is responsible for any snapshot. */
474+
sysscan->snapshot=NULL;
475+
}
476+
447477
/* Change attribute numbers to be index column numbers. */
448478
for (i=0;i<nkeys;i++)
449479
{
@@ -494,5 +524,7 @@ systable_endscan_ordered(SysScanDesc sysscan)
494524
{
495525
Assert(sysscan->irel);
496526
index_endscan(sysscan->iscan);
527+
if (sysscan->snapshot)
528+
UnregisterSnapshot(sysscan->snapshot);
497529
pfree(sysscan);
498530
}

‎src/backend/access/nbtree/README‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,10 @@ deletes index entries before deleting tuples, the super-exclusive lock
141141
guarantees that VACUUM can't delete any heap tuple that an indexscanning
142142
process might be about to visit. (This guarantee works only for simple
143143
indexscans that visit the heap in sync with the index scan, not for bitmap
144-
scans. We only need the guarantee when using non-MVCC snapshot rules such
145-
as SnapshotNow, so in practice this is only important for system catalog
146-
accesses.)
144+
scans. We only need the guarantee when using non-MVCC snapshot rules; in
145+
an MVCC snapshot, it wouldn't matter if the heap tuple were replaced with
146+
an unrelated tuple at the same TID, because the new tuple wouldn't be
147+
visible to our scan anyway.)
147148

148149
Because a page can be split even while someone holds a pin on it, it is
149150
possible that an indexscan will return items that are no longer stored on

‎src/backend/access/rmgrdesc/xactdesc.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,14 @@ xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
6969
appendStringInfo(buf," catalog %u",msg->cat.catId);
7070
elseif (msg->id==SHAREDINVALRELCACHE_ID)
7171
appendStringInfo(buf," relcache %u",msg->rc.relId);
72-
/*remaining casesnot expected, but print something anyway */
72+
/* not expected, but print something anyway */
7373
elseif (msg->id==SHAREDINVALSMGR_ID)
7474
appendStringInfo(buf," smgr");
75+
/* not expected, but print something anyway */
7576
elseif (msg->id==SHAREDINVALRELMAP_ID)
7677
appendStringInfo(buf," relmap");
78+
elseif (msg->id==SHAREDINVALSNAPSHOT_ID)
79+
appendStringInfo(buf," snapshot %u",msg->sn.relId);
7780
else
7881
appendStringInfo(buf," unknown id %d",msg->id);
7982
}

‎src/backend/bootstrap/bootstrap.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ boot_openrel(char *relname)
611611
{
612612
/* We can now load the pg_type data */
613613
rel=heap_open(TypeRelationId,NoLock);
614-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
614+
scan=heap_beginscan_catalog(rel,0,NULL);
615615
i=0;
616616
while ((tup=heap_getnext(scan,ForwardScanDirection))!=NULL)
617617
++i;
@@ -620,7 +620,7 @@ boot_openrel(char *relname)
620620
while (i-->0)
621621
*app++=ALLOC(structtypmap,1);
622622
*app=NULL;
623-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
623+
scan=heap_beginscan_catalog(rel,0,NULL);
624624
app=Typ;
625625
while ((tup=heap_getnext(scan,ForwardScanDirection))!=NULL)
626626
{
@@ -918,7 +918,7 @@ gettype(char *type)
918918
}
919919
elog(DEBUG4,"external type: %s",type);
920920
rel=heap_open(TypeRelationId,NoLock);
921-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
921+
scan=heap_beginscan_catalog(rel,0,NULL);
922922
i=0;
923923
while ((tup=heap_getnext(scan,ForwardScanDirection))!=NULL)
924924
++i;
@@ -927,7 +927,7 @@ gettype(char *type)
927927
while (i-->0)
928928
*app++=ALLOC(structtypmap,1);
929929
*app=NULL;
930-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
930+
scan=heap_beginscan_catalog(rel,0,NULL);
931931
app=Typ;
932932
while ((tup=heap_getnext(scan,ForwardScanDirection))!=NULL)
933933
{

‎src/backend/catalog/aclchk.c‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ objectsInSchemaToOids(GrantObjectType objtype, List *nspnames)
788788
ObjectIdGetDatum(namespaceId));
789789

790790
rel=heap_open(ProcedureRelationId,AccessShareLock);
791-
scan=heap_beginscan(rel,SnapshotNow,1,key);
791+
scan=heap_beginscan_catalog(rel,1,key);
792792

793793
while ((tuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
794794
{
@@ -833,7 +833,7 @@ getRelationsInNamespace(Oid namespaceId, char relkind)
833833
CharGetDatum(relkind));
834834

835835
rel=heap_open(RelationRelationId,AccessShareLock);
836-
scan=heap_beginscan(rel,SnapshotNow,2,key);
836+
scan=heap_beginscan_catalog(rel,2,key);
837837

838838
while ((tuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
839839
{
@@ -1332,7 +1332,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
13321332
ObjectIdGetDatum(objid));
13331333

13341334
scan=systable_beginscan(rel,DefaultAclOidIndexId, true,
1335-
SnapshotNow,1,skey);
1335+
NULL,1,skey);
13361336

13371337
tuple=systable_getnext(scan);
13381338

@@ -1452,7 +1452,7 @@ RemoveDefaultACLById(Oid defaclOid)
14521452
ObjectIdGetDatum(defaclOid));
14531453

14541454
scan=systable_beginscan(rel,DefaultAclOidIndexId, true,
1455-
SnapshotNow,1,skey);
1455+
NULL,1,skey);
14561456

14571457
tuple=systable_getnext(scan);
14581458

@@ -2705,7 +2705,7 @@ ExecGrant_Largeobject(InternalGrant *istmt)
27052705

27062706
scan=systable_beginscan(relation,
27072707
LargeObjectMetadataOidIndexId, true,
2708-
SnapshotNow,1,entry);
2708+
NULL,1,entry);
27092709

27102710
tuple=systable_getnext(scan);
27112711
if (!HeapTupleIsValid(tuple))
@@ -3468,7 +3468,7 @@ pg_aclmask(AclObjectKind objkind, Oid table_oid, AttrNumber attnum, Oid roleid,
34683468
returnpg_language_aclmask(table_oid,roleid,mask,how);
34693469
caseACL_KIND_LARGEOBJECT:
34703470
returnpg_largeobject_aclmask_snapshot(table_oid,roleid,
3471-
mask,how,SnapshotNow);
3471+
mask,how,NULL);
34723472
caseACL_KIND_NAMESPACE:
34733473
returnpg_namespace_aclmask(table_oid,roleid,mask,how);
34743474
caseACL_KIND_TABLESPACE:
@@ -3856,10 +3856,13 @@ pg_language_aclmask(Oid lang_oid, Oid roleid,
38563856
* Exported routine for examining a user's privileges for a largeobject
38573857
*
38583858
* When a large object is opened for reading, it is opened relative to the
3859-
* caller's snapshot, but when it is opened for writing, it is always relative
3860-
* to SnapshotNow, as documented in doc/src/sgml/lobj.sgml. This function
3861-
* takes a snapshot argument so that the permissions check can be made relative
3862-
* to the same snapshot that will be used to read the underlying data.
3859+
* caller's snapshot, but when it is opened for writing, a current
3860+
* MVCC snapshot will be used. See doc/src/sgml/lobj.sgml. This function
3861+
* takes a snapshot argument so that the permissions check can be made
3862+
* relative to the same snapshot that will be used to read the underlying
3863+
* data. The caller will actually pass NULL for an instantaneous MVCC
3864+
* snapshot, since all we do with the snapshot argument is pass it through
3865+
* to systable_beginscan().
38633866
*/
38643867
AclMode
38653868
pg_largeobject_aclmask_snapshot(Oidlobj_oid,Oidroleid,
@@ -4644,7 +4647,7 @@ pg_language_ownercheck(Oid lan_oid, Oid roleid)
46444647
* Ownership check for a largeobject (specified by OID)
46454648
*
46464649
* This is only used for operations like ALTER LARGE OBJECT that are always
4647-
* relative toSnapshotNow.
4650+
* relative toan up-to-date snapshot.
46484651
*/
46494652
bool
46504653
pg_largeobject_ownercheck(Oidlobj_oid,Oidroleid)
@@ -4670,7 +4673,7 @@ pg_largeobject_ownercheck(Oid lobj_oid, Oid roleid)
46704673

46714674
scan=systable_beginscan(pg_lo_meta,
46724675
LargeObjectMetadataOidIndexId, true,
4673-
SnapshotNow,1,entry);
4676+
NULL,1,entry);
46744677

46754678
tuple=systable_getnext(scan);
46764679
if (!HeapTupleIsValid(tuple))
@@ -5032,7 +5035,7 @@ pg_extension_ownercheck(Oid ext_oid, Oid roleid)
50325035

50335036
scan=systable_beginscan(pg_extension,
50345037
ExtensionOidIndexId, true,
5035-
SnapshotNow,1,entry);
5038+
NULL,1,entry);
50365039

50375040
tuple=systable_getnext(scan);
50385041
if (!HeapTupleIsValid(tuple))

‎src/backend/catalog/catalog.c‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -218,20 +218,16 @@ IsReservedName(const char *name)
218218
*Given the OID of a relation, determine whether it's supposed to be
219219
*shared across an entire database cluster.
220220
*
221-
* Hard-wiring this list is pretty grotty, but we really need it so that
222-
* we can compute the locktag for a relation (and then lock it) without
223-
* having already read its pg_class entry.If we try to retrieve relisshared
224-
* from pg_class with no pre-existing lock, there is a race condition against
225-
* anyone who is concurrently committing a change to the pg_class entry:
226-
* since we read system catalog entries under SnapshotNow, it's possible
227-
* that both the old and new versions of the row are invalid at the instants
228-
* we scan them. We fix this by insisting that updaters of a pg_class
229-
* row must hold exclusive lock on the corresponding rel, and that users
230-
* of a relation must hold at least AccessShareLock on the rel *before*
231-
* trying to open its relcache entry. But to lock a rel, you have to
232-
* know if it's shared. Fortunately, the set of shared relations is
233-
* fairly static, so a hand-maintained list of their OIDs isn't completely
234-
* impractical.
221+
* In older releases, this had to be hard-wired so that we could compute the
222+
* locktag for a relation and lock it before examining its catalog entry.
223+
* Since we now have MVCC catalog access, the race conditions that made that
224+
* a hard requirement are gone, so we could look at relaxing this restriction.
225+
* However, if we scanned the pg_class entry to find relisshared, and only
226+
* then locked the relation, pg_class could get updated in the meantime,
227+
* forcing us to scan the relation again, which would definitely be complex
228+
* and might have undesirable performance consequences. Fortunately, the set
229+
* of shared relations is fairly static, so a hand-maintained list of their
230+
* OIDs isn't completely impractical.
235231
*/
236232
bool
237233
IsSharedRelation(OidrelationId)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp