diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index c9b80ad8232..5662d3f1a63 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -129,6 +129,7 @@ createdb(const CreatedbStmt *stmt) int notherbackends; int npreparedxacts; createdb_failure_params fparms; + Snapshot snapshot; /* Extract options from the statement node tree */ foreach(option, stmt->options) @@ -532,6 +533,29 @@ createdb(const CreatedbStmt *stmt) */ RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_FORCE | CHECKPOINT_WAIT); + /* + * Take an MVCC snapshot to use while scanning through pg_tablespace. For + * safety, register the snapshot (this prevents it from changing if + * something else were to request a snapshot during the loop). + * + * Traversing pg_tablespace with an MVCC snapshot is necessary to provide + * us with a consistent view of the tablespaces that exist. Using + * SnapshotNow here would risk seeing the same tablespace multiple times, + * or worse not seeing a tablespace at all, if its tuple is moved around + * by a concurrent update (eg an ACL change). + * + * Inconsistency of this sort is inherent to all SnapshotNow scans, unless + * some lock is held to prevent concurrent updates of the rows being + * sought. There should be a generic fix for that, but in the meantime + * it's worth fixing this case in particular because we are doing very + * heavyweight operations within the scan, so that the elapsed time for + * the scan is vastly longer than for most other catalog scans. That + * means there's a much wider window for concurrent updates to cause + * trouble here than anywhere else. XXX this code should be changed + * whenever a generic fix is implemented. + */ + snapshot = RegisterSnapshot(GetLatestSnapshot()); + /* * Once we start copying subdirectories, we need to be able to clean 'em * up if we fail. Use an ENSURE block to make sure this happens. (This @@ -549,7 +573,7 @@ createdb(const CreatedbStmt *stmt) * each one to the new database. */ rel = heap_open(TableSpaceRelationId, AccessShareLock); - scan = heap_beginscan(rel, SnapshotNow, 0, NULL); + scan = heap_beginscan(rel, snapshot, 0, NULL); while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { Oid srctablespace = HeapTupleGetOid(tuple); @@ -653,6 +677,9 @@ createdb(const CreatedbStmt *stmt) } PG_END_ENSURE_ERROR_CLEANUP(createdb_failure_callback, PointerGetDatum(&fparms)); + + /* Free our snapshot */ + UnregisterSnapshot(snapshot); } /* @@ -1698,9 +1725,20 @@ remove_dbtablespaces(Oid db_id) Relation rel; HeapScanDesc scan; HeapTuple tuple; + Snapshot snapshot; + + /* + * As in createdb(), we'd better use an MVCC snapshot here, since this + * scan can run for a long time. Duplicate visits to tablespaces would be + * harmless, but missing a tablespace could result in permanently leaked + * files. + * + * XXX change this when a generic fix for SnapshotNow races is implemented + */ + snapshot = RegisterSnapshot(GetLatestSnapshot()); rel = heap_open(TableSpaceRelationId, AccessShareLock); - scan = heap_beginscan(rel, SnapshotNow, 0, NULL); + scan = heap_beginscan(rel, snapshot, 0, NULL); while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { Oid dsttablespace = HeapTupleGetOid(tuple); @@ -1746,6 +1784,7 @@ remove_dbtablespaces(Oid db_id) heap_endscan(scan); heap_close(rel, AccessShareLock); + UnregisterSnapshot(snapshot); } /* @@ -1767,9 +1806,19 @@ check_db_file_conflict(Oid db_id) Relation rel; HeapScanDesc scan; HeapTuple tuple; + Snapshot snapshot; + + /* + * As in createdb(), we'd better use an MVCC snapshot here; missing a + * tablespace could result in falsely reporting the OID is unique, with + * disastrous future consequences per the comment above. + * + * XXX change this when a generic fix for SnapshotNow races is implemented + */ + snapshot = RegisterSnapshot(GetLatestSnapshot()); rel = heap_open(TableSpaceRelationId, AccessShareLock); - scan = heap_beginscan(rel, SnapshotNow, 0, NULL); + scan = heap_beginscan(rel, snapshot, 0, NULL); while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { Oid dsttablespace = HeapTupleGetOid(tuple); @@ -1795,6 +1844,8 @@ check_db_file_conflict(Oid db_id) heap_endscan(scan); heap_close(rel, AccessShareLock); + UnregisterSnapshot(snapshot); + return result; }