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

Commitc54ebcb

Browse files
committed
Protect against SnapshotNow race conditions in pg_tablespace scans.
Use of SnapshotNow is known to expose us to race conditions if the tuple(s)being sought could be updated by concurrently-committing transactions.CREATE DATABASE and DROP DATABASE are particularly exposed because they doheavyweight filesystem operations during their scans of pg_tablespace,so that the scans run for a very long time compared to most. Furthermore,the potential consequences of a missed or twice-visited row are nastierthan average:* createdb() could fail with a bogus "file already exists" error, or silently fail to copy one or more tablespace's worth of files into the new database.* remove_dbtablespaces() could miss one or more tablespaces, thus failing to free filesystem space for the dropped database.* check_db_file_conflict() could likewise miss a tablespace, leading to an OID conflict that could result in data loss either immediately or in future operations. (This seems of very low probability, though, since a duplicate database OID would be unlikely to start with.)Hence, it seems worth fixing these three places to use MVCC snapshots, eventhough this will someday be superseded by a generic solution to SnapshotNowrace conditions.Back-patch to all active branches.Stephen Frost and Tom Lane
1 parent66debec commitc54ebcb

File tree

1 file changed

+54
-3
lines changed

1 file changed

+54
-3
lines changed

‎src/backend/commands/dbcommands.c

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ createdb(const CreatedbStmt *stmt)
132132
intnotherbackends;
133133
intnpreparedxacts;
134134
createdb_failure_paramsfparms;
135+
Snapshotsnapshot;
135136

136137
/* Extract options from the statement node tree */
137138
foreach(option,stmt->options)
@@ -532,6 +533,29 @@ createdb(const CreatedbStmt *stmt)
532533
*/
533534
RequestCheckpoint(CHECKPOINT_IMMEDIATE |CHECKPOINT_FORCE |CHECKPOINT_WAIT);
534535

536+
/*
537+
* Take an MVCC snapshot to use while scanning through pg_tablespace. For
538+
* safety, register the snapshot (this prevents it from changing if
539+
* something else were to request a snapshot during the loop).
540+
*
541+
* Traversing pg_tablespace with an MVCC snapshot is necessary to provide
542+
* us with a consistent view of the tablespaces that exist. Using
543+
* SnapshotNow here would risk seeing the same tablespace multiple times,
544+
* or worse not seeing a tablespace at all, if its tuple is moved around
545+
* by a concurrent update (eg an ACL change).
546+
*
547+
* Inconsistency of this sort is inherent to all SnapshotNow scans, unless
548+
* some lock is held to prevent concurrent updates of the rows being
549+
* sought.There should be a generic fix for that, but in the meantime
550+
* it's worth fixing this case in particular because we are doing very
551+
* heavyweight operations within the scan, so that the elapsed time for
552+
* the scan is vastly longer than for most other catalog scans. That
553+
* means there's a much wider window for concurrent updates to cause
554+
* trouble here than anywhere else. XXX this code should be changed
555+
* whenever a generic fix is implemented.
556+
*/
557+
snapshot=RegisterSnapshot(GetLatestSnapshot());
558+
535559
/*
536560
* Once we start copying subdirectories, we need to be able to clean 'em
537561
* up if we fail. Use an ENSURE block to make sure this happens. (This
@@ -549,7 +573,7 @@ createdb(const CreatedbStmt *stmt)
549573
* each one to the new database.
550574
*/
551575
rel=heap_open(TableSpaceRelationId,AccessShareLock);
552-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
576+
scan=heap_beginscan(rel,snapshot,0,NULL);
553577
while ((tuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
554578
{
555579
Oidsrctablespace=HeapTupleGetOid(tuple);
@@ -653,6 +677,9 @@ createdb(const CreatedbStmt *stmt)
653677
}
654678
PG_END_ENSURE_ERROR_CLEANUP(createdb_failure_callback,
655679
PointerGetDatum(&fparms));
680+
681+
/* Free our snapshot */
682+
UnregisterSnapshot(snapshot);
656683
}
657684

658685
/*
@@ -1687,9 +1714,20 @@ remove_dbtablespaces(Oid db_id)
16871714
Relationrel;
16881715
HeapScanDescscan;
16891716
HeapTupletuple;
1717+
Snapshotsnapshot;
1718+
1719+
/*
1720+
* As in createdb(), we'd better use an MVCC snapshot here, since this
1721+
* scan can run for a long time. Duplicate visits to tablespaces would be
1722+
* harmless, but missing a tablespace could result in permanently leaked
1723+
* files.
1724+
*
1725+
* XXX change this when a generic fix for SnapshotNow races is implemented
1726+
*/
1727+
snapshot=RegisterSnapshot(GetLatestSnapshot());
16901728

16911729
rel=heap_open(TableSpaceRelationId,AccessShareLock);
1692-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
1730+
scan=heap_beginscan(rel,snapshot,0,NULL);
16931731
while ((tuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
16941732
{
16951733
Oiddsttablespace=HeapTupleGetOid(tuple);
@@ -1735,6 +1773,7 @@ remove_dbtablespaces(Oid db_id)
17351773

17361774
heap_endscan(scan);
17371775
heap_close(rel,AccessShareLock);
1776+
UnregisterSnapshot(snapshot);
17381777
}
17391778

17401779
/*
@@ -1756,9 +1795,19 @@ check_db_file_conflict(Oid db_id)
17561795
Relationrel;
17571796
HeapScanDescscan;
17581797
HeapTupletuple;
1798+
Snapshotsnapshot;
1799+
1800+
/*
1801+
* As in createdb(), we'd better use an MVCC snapshot here; missing a
1802+
* tablespace could result in falsely reporting the OID is unique, with
1803+
* disastrous future consequences per the comment above.
1804+
*
1805+
* XXX change this when a generic fix for SnapshotNow races is implemented
1806+
*/
1807+
snapshot=RegisterSnapshot(GetLatestSnapshot());
17591808

17601809
rel=heap_open(TableSpaceRelationId,AccessShareLock);
1761-
scan=heap_beginscan(rel,SnapshotNow,0,NULL);
1810+
scan=heap_beginscan(rel,snapshot,0,NULL);
17621811
while ((tuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
17631812
{
17641813
Oiddsttablespace=HeapTupleGetOid(tuple);
@@ -1784,6 +1833,8 @@ check_db_file_conflict(Oid db_id)
17841833

17851834
heap_endscan(scan);
17861835
heap_close(rel,AccessShareLock);
1836+
UnregisterSnapshot(snapshot);
1837+
17871838
returnresult;
17881839
}
17891840

0 commit comments

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