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

Commit09d3670

Browse files
committed
Change the relation_open protocol so that we obtain lock on a relation
(table or index) before trying to open its relcache entry. This fixesrace conditions in which someone else commits a change to the relation'scatalog entries while we are in process of doing relcache load. Problemsof that ilk have been reported sporadically for years, but it was notreally practical to fix until recently --- for instance, the recentaddition of WAL-log support for in-place updates helped.Along the way, remove pg_am.amconcurrent: all AMs are now expected to supportconcurrent update.
1 parent4cd72b5 commit09d3670

File tree

40 files changed

+629
-642
lines changed

40 files changed

+629
-642
lines changed

‎contrib/userlock/user_locks.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ user_lock(uint32 id1, uint32 id2, LOCKMODE lockmode)
3535

3636
SET_LOCKTAG_USERLOCK(tag,id1,id2);
3737

38-
return (LockAcquire(&tag, false,
39-
lockmode, true, true)!=LOCKACQUIRE_NOT_AVAIL);
38+
return (LockAcquire(&tag,lockmode, true, true)!=LOCKACQUIRE_NOT_AVAIL);
4039
}
4140

4241
int

‎doc/src/sgml/catalogs.sgml

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.128 2006/07/2708:30:41 petere Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.129 2006/07/31 20:08:55 tgl Exp $ -->
22
<!--
33
Documentation of the system catalogs, directed toward PostgreSQL developers
44
-->
@@ -401,13 +401,6 @@
401401
<entry>Can index storage data type differ from column data type?</entry>
402402
</row>
403403

404-
<row>
405-
<entry><structfield>amconcurrent</structfield></entry>
406-
<entry><type>bool</type></entry>
407-
<entry></entry>
408-
<entry>Does the access method support concurrent updates?</entry>
409-
</row>
410-
411404
<row>
412405
<entry><structfield>amclusterable</structfield></entry>
413406
<entry><type>bool</type></entry>

‎doc/src/sgml/indexam.sgml

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.15 2006/07/03 22:45:36 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.16 2006/07/31 20:08:59 tgl Exp $ -->
22

33
<chapter id="indexam">
44
<title>Index Access Method Interface Definition</title>
@@ -94,8 +94,7 @@
9494
<para>
9595
Some of the flag columns of <structname>pg_am</structname> have nonobvious
9696
implications. The requirements of <structfield>amcanunique</structfield>
97-
are discussed in <xref linkend="index-unique-checks">, and those of
98-
<structfield>amconcurrent</structfield> in <xref linkend="index-locking">.
97+
are discussed in <xref linkend="index-unique-checks">.
9998
The <structfield>amcanmulticol</structfield> flag asserts that the
10099
access method supports multicolumn indexes, while
101100
<structfield>amoptionalkey</structfield> asserts that it allows scans
@@ -474,11 +473,7 @@ amrestrpos (IndexScanDesc scan);
474473
a concurrent delete may or may not be reflected in the results of a scan.
475474
What is important is that insertions or deletions not cause the scan to
476475
miss or multiply return entries that were not themselves being inserted or
477-
deleted. (For an index type that does not set
478-
<structname>pg_am</>.<structfield>amconcurrent</>, it is sufficient to
479-
handle these cases for insertions or deletions performed by the same
480-
backend that's doing the scan. But when <structfield>amconcurrent</> is
481-
true, insertions or deletions from other backends must be handled as well.)
476+
deleted.
482477
</para>
483478

484479
<para>
@@ -506,31 +501,16 @@ amrestrpos (IndexScanDesc scan);
506501
<title>Index Locking Considerations</title>
507502

508503
<para>
509-
An index access method can choose whether it supports concurrent updates
510-
of the index by multiple processes. If the method's
511-
<structname>pg_am</>.<structfield>amconcurrent</> flag is true, then
512-
the core <productname>PostgreSQL</productname> system obtains
504+
Index access methods must handle concurrent updates
505+
of the index by multiple processes.
506+
The core <productname>PostgreSQL</productname> system obtains
513507
<literal>AccessShareLock</> on the index during an index scan, and
514-
<literal>RowExclusiveLock</> when updating the index. Since these lock
508+
<literal>RowExclusiveLock</> when updating the index (including plain
509+
<command>VACUUM</>). Since these lock
515510
types do not conflict, the access method is responsible for handling any
516511
fine-grained locking it may need. An exclusive lock on the index as a whole
517-
will be taken only during index creation, destruction, or
518-
<literal>REINDEX</>. When <structfield>amconcurrent</> is false,
519-
<productname>PostgreSQL</productname> still obtains
520-
<literal>AccessShareLock</> during index scans, but it obtains
521-
<literal>AccessExclusiveLock</> during any update. This ensures that
522-
updaters have sole use of the index. Note that this implicitly assumes
523-
that index scans are read-only; an access method that might modify the
524-
index during a scan will still have to do its own locking to handle the
525-
case of concurrent scans.
526-
</para>
527-
528-
<para>
529-
Recall that a backend's own locks never conflict; therefore, even a
530-
non-concurrent index type must be prepared to handle the case where
531-
a backend is inserting or deleting entries in an index that it is itself
532-
scanning. (This is of course necessary to support an <command>UPDATE</>
533-
that uses the index to find the rows to be updated.)
512+
will be taken only during index creation, destruction,
513+
<command>REINDEX</>, or <command>VACUUM FULL</>.
534514
</para>
535515

536516
<para>
@@ -567,7 +547,7 @@ amrestrpos (IndexScanDesc scan);
567547
</listitem>
568548
<listitem>
569549
<para>
570-
For concurrent index types, an index scan must maintain a pin
550+
An index scan must maintain a pin
571551
on the index page holding the item last returned by
572552
<function>amgettuple</>, and <function>ambulkdelete</> cannot delete
573553
entries from pages that are pinned by other backends. The need
@@ -576,11 +556,10 @@ amrestrpos (IndexScanDesc scan);
576556
</listitem>
577557
</itemizedlist>
578558

579-
If an index is concurrent then it is possible for an index reader to
559+
Without the third rule, it is possible for an index reader to
580560
see an index entry just before it is removed by <command>VACUUM</>, and
581561
then to arrive at the corresponding heap entry after that was removed by
582-
<command>VACUUM</>. (With a nonconcurrent index, this is not possible
583-
because of the conflicting index-level locks that will be taken out.)
562+
<command>VACUUM</>.
584563
This creates no serious problems if that item
585564
number is still unused when the reader reaches it, since an empty
586565
item slot will be ignored by <function>heap_fetch()</>. But what if a

‎src/backend/access/gin/ginvacuum.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.4 2006/07/14 14:52:16 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.5 2006/07/31 20:08:59 tgl Exp $
1212
*-------------------------------------------------------------------------
1313
*/
1414

@@ -572,7 +572,7 @@ ginvacuumcleanup(PG_FUNCTION_ARGS) {
572572
IndexVacuumInfo*info= (IndexVacuumInfo*)PG_GETARG_POINTER(0);
573573
IndexBulkDeleteResult*stats= (IndexBulkDeleteResult*)PG_GETARG_POINTER(1);
574574
Relationindex=info->index;
575-
boolneedLock= !RELATION_IS_LOCAL(index);
575+
boolneedLock;
576576
BlockNumbernpages,
577577
blkno;
578578
BlockNumbernFreePages,
@@ -591,10 +591,14 @@ ginvacuumcleanup(PG_FUNCTION_ARGS) {
591591
*/
592592
stats->num_index_tuples=info->num_heap_tuples;
593593

594-
if (info->vacuum_full) {
595-
LockRelation(index,AccessExclusiveLock);
594+
/*
595+
* If vacuum full, we already have exclusive lock on the index.
596+
* Otherwise, need lock unless it's local to this backend.
597+
*/
598+
if (info->vacuum_full)
596599
needLock= false;
597-
}
600+
else
601+
needLock= !RELATION_IS_LOCAL(index);
598602

599603
if (needLock)
600604
LockRelationForExtension(index,ExclusiveLock);
@@ -653,9 +657,6 @@ ginvacuumcleanup(PG_FUNCTION_ARGS) {
653657
if (needLock)
654658
UnlockRelationForExtension(index,ExclusiveLock);
655659

656-
if (info->vacuum_full)
657-
UnlockRelation(index,AccessExclusiveLock);
658-
659660
PG_RETURN_POINTER(stats);
660661
}
661662

‎src/backend/access/gist/gistvacuum.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.25 2006/07/14 14:52:16 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.26 2006/07/31 20:08:59 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -517,7 +517,7 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
517517
GistVacuumgv;
518518
ArrayTupleres;
519519

520-
LockRelation(rel,AccessExclusiveLock);
520+
/* note: vacuum.c already acquiredAccessExclusiveLock on index */
521521

522522
gv.index=rel;
523523
initGISTstate(&(gv.giststate),rel);
@@ -543,8 +543,12 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
543543
(errmsg("index \"%s\" needs VACUUM FULL or REINDEX to finish crash recovery",
544544
RelationGetRelationName(rel))));
545545

546+
/*
547+
* If vacuum full, we already have exclusive lock on the index.
548+
* Otherwise, need lock unless it's local to this backend.
549+
*/
546550
if (info->vacuum_full)
547-
needLock= false;/* relation locked with AccessExclusiveLock */
551+
needLock= false;
548552
else
549553
needLock= !RELATION_IS_LOCAL(rel);
550554

@@ -613,9 +617,6 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
613617
if (needLock)
614618
UnlockRelationForExtension(rel,ExclusiveLock);
615619

616-
if (info->vacuum_full)
617-
UnlockRelation(rel,AccessExclusiveLock);
618-
619620
PG_RETURN_POINTER(stats);
620621
}
621622

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

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.217 2006/07/14 14:52:17 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.218 2006/07/31 20:08:59 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -51,6 +51,7 @@
5151
#include"pgstat.h"
5252
#include"storage/procarray.h"
5353
#include"utils/inval.h"
54+
#include"utils/lsyscache.h"
5455
#include"utils/relcache.h"
5556

5657

@@ -687,15 +688,16 @@ relation_open(Oid relationId, LOCKMODE lockmode)
687688

688689
Assert(lockmode >=NoLock&&lockmode<MAX_LOCKMODES);
689690

691+
/* Get the lock before trying to open the relcache entry */
692+
if (lockmode!=NoLock)
693+
LockRelationOid(relationId,lockmode);
694+
690695
/* The relcache does all the real work... */
691696
r=RelationIdGetRelation(relationId);
692697

693698
if (!RelationIsValid(r))
694699
elog(ERROR,"could not open relation with OID %u",relationId);
695700

696-
if (lockmode!=NoLock)
697-
LockRelation(r,lockmode);
698-
699701
returnr;
700702
}
701703

@@ -713,26 +715,38 @@ conditional_relation_open(Oid relationId, LOCKMODE lockmode, bool nowait)
713715

714716
Assert(lockmode >=NoLock&&lockmode<MAX_LOCKMODES);
715717

716-
/* The relcache does all the real work... */
717-
r=RelationIdGetRelation(relationId);
718-
719-
if (!RelationIsValid(r))
720-
elog(ERROR,"could not open relation with OID %u",relationId);
721-
718+
/* Get the lock before trying to open the relcache entry */
722719
if (lockmode!=NoLock)
723720
{
724721
if (nowait)
725722
{
726-
if (!ConditionalLockRelation(r,lockmode))
727-
ereport(ERROR,
728-
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
729-
errmsg("could not obtain lock on relation \"%s\"",
730-
RelationGetRelationName(r))));
723+
if (!ConditionalLockRelationOid(relationId,lockmode))
724+
{
725+
/* try to throw error by name; relation could be deleted... */
726+
char*relname=get_rel_name(relationId);
727+
728+
if (relname)
729+
ereport(ERROR,
730+
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
731+
errmsg("could not obtain lock on relation \"%s\"",
732+
relname)));
733+
else
734+
ereport(ERROR,
735+
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
736+
errmsg("could not obtain lock on relation with OID %u",
737+
relationId)));
738+
}
731739
}
732740
else
733-
LockRelation(r,lockmode);
741+
LockRelationOid(relationId,lockmode);
734742
}
735743

744+
/* The relcache does all the real work... */
745+
r=RelationIdGetRelation(relationId);
746+
747+
if (!RelationIsValid(r))
748+
elog(ERROR,"could not open relation with OID %u",relationId);
749+
736750
returnr;
737751
}
738752

@@ -749,12 +763,12 @@ relation_openrv(const RangeVar *relation, LOCKMODE lockmode)
749763

750764
/*
751765
* Check for shared-cache-inval messages before trying to open the
752-
* relation. This is needed to cover the case where the name identifies a
753-
* rel that has been dropped and recreated since the start of our
766+
* relation. This is needed to cover the case where the name identifies
767+
*arel that has been dropped and recreated since the start of our
754768
* transaction: if we don't flush the old syscache entry then we'll latch
755-
* onto that entry and suffer an error when we doLockRelation. Note that
756-
* relation_open does not need to do this, since a relation's OID never
757-
* changes.
769+
* onto that entry and suffer an error when we doRelationIdGetRelation.
770+
*Note thatrelation_open does not need to do this, since a relation's
771+
*OID neverchanges.
758772
*
759773
* We skip this if asked for NoLock, on the assumption that the caller has
760774
* already ensured some appropriate lock is held.
@@ -772,7 +786,7 @@ relation_openrv(const RangeVar *relation, LOCKMODE lockmode)
772786
/* ----------------
773787
*relation_close - close any relation
774788
*
775-
*If lockmode is not "NoLock", wefirst release the specified lock.
789+
*If lockmode is not "NoLock", wethen release the specified lock.
776790
*
777791
*Note that it is often sensible to hold a lock beyond relation_close;
778792
*in that case, the lock is released automatically at xact end.
@@ -781,13 +795,15 @@ relation_openrv(const RangeVar *relation, LOCKMODE lockmode)
781795
void
782796
relation_close(Relationrelation,LOCKMODElockmode)
783797
{
784-
Assert(lockmode >=NoLock&&lockmode<MAX_LOCKMODES);
798+
LockRelIdrelid=relation->rd_lockInfo.lockRelId;
785799

786-
if (lockmode!=NoLock)
787-
UnlockRelation(relation,lockmode);
800+
Assert(lockmode >=NoLock&&lockmode<MAX_LOCKMODES);
788801

789802
/* The relcache does the real work... */
790803
RelationClose(relation);
804+
805+
if (lockmode!=NoLock)
806+
UnlockRelationId(&relid,lockmode);
791807
}
792808

793809

0 commit comments

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