Prevent CREATE TABLE from using dangling tablespace (#876)

gfphoenix78 · web-flow · commit df1e2ff5ae7e · 2025-01-20T09:38:49.000+08:00
When DROP TABLESPACE is running, it's possible to still use the dropping
tablespace in CREATE TABLE. It's bad behavior that the table may use
a dropped tablespace, which means that data files are stored out of
the database.

This commit controls creating files in a tablespace and dropping tablespace
running in exclusive mode. The key timelines for dropping a tablespace is:
1. Lock tuple of pg_tablespace in AccessExclusive mode.
2. CommitTransaction.
3. Remove directories of the tablespace.
4. Release the lock of the tablespace tuple.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
@@ -2995,6 +2995,8 @@ CommitTransaction(void)
 
 	AtEOXact_MultiXact();
 
+	AtCommit_TablespaceStorage();
+
 	ResourceOwnerRelease(TopTransactionResourceOwner,
 						 RESOURCE_RELEASE_LOCKS,
 						 true, true);
@@ -3026,8 +3028,6 @@ CommitTransaction(void)
 	if(Gp_role == GP_ROLE_DISPATCH || IS_SINGLENODE())
 		MoveDbSessionLockRelease();
 
-	AtCommit_TablespaceStorage();
-
 	/*
 	 * Send out notification signals to other backends (and do other
 	 * post-commit NOTIFY cleanup).  This must not happen until after our
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
@@ -123,6 +123,24 @@ static void ensure_tablespace_directory_is_empty(const Oid tablespaceoid, const
 static void unlink_during_redo(Oid tablepace_oid_to_unlink);
 static void unlink_without_redo(Oid tablespace_oid_to_unlink);
 
+static bool
+TablespaceLockTuple(Oid tablespace_oid, LOCKMODE lockmode, bool wait)
+{
+	bool ok = true;
+
+	Assert(OidIsValid(tablespace_oid));
+	Assert(tablespace_oid != GLOBALTABLESPACE_OID);
+	Assert(tablespace_oid != DEFAULTTABLESPACE_OID);
+
+	if (wait)
+		LockSharedObject(TableSpaceRelationId, tablespace_oid, 0, lockmode);
+	else
+		ok = ConditionalLockSharedObject(TableSpaceRelationId, tablespace_oid,
+										 0, lockmode);
+
+	return ok;
+}
+
 /*
  * Each database using a table space is isolated into its own name space
  * by a subdirectory named for the database OID.  On first creation of an
@@ -156,6 +174,9 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
 	Assert(OidIsValid(spcNode));
 	Assert(OidIsValid(dbNode));
 
+	if (spcNode != DEFAULTTABLESPACE_OID && !isRedo)
+		TablespaceLockTuple(spcNode, AccessShareLock, true);
+
 	dir = GetDatabasePath(dbNode, spcNode);
 
 	if (stat(dir, &st) < 0)
@@ -720,6 +741,13 @@ DropTableSpace(DropTableSpaceStmt *stmt)
 	/* DROP hook for the tablespace being removed */
 	InvokeObjectDropHook(TableSpaceRelationId, tablespaceoid, 0);
 
+	if (!TablespaceLockTuple(tablespaceoid, AccessExclusiveLock, false))
+		ereport(ERROR,
+				(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
+				 errmsg("could not lock tablespace \"%s\"",
+						tablespacename)));
+	SIMPLE_FAULT_INJECTOR("drop_tablespace_after_acquire_lock");
+
 	/*
 	 * Remove the pg_tablespace tuple (this will roll back if we fail below)
 	 */
@@ -795,7 +823,6 @@ DropTableSpace(DropTableSpaceStmt *stmt)
 
 	/* We keep the lock on pg_tablespace until commit */
 	table_close(rel, NoLock);
-	SIMPLE_FAULT_INJECTOR("AfterTablespaceCreateLockRelease");
 
 	/*
 	 * If we are the QD, dispatch this DROP command to all the QEs
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
@@ -1153,6 +1153,27 @@ LockSharedObject(Oid classid, Oid objid, uint16 objsubid,
 	AcceptInvalidationMessages();
 }
 
+/*
+ *		ConditionalLockSharedObject
+ *
+ * As above, but only lock if we can get the lock without blocking.
+ * Returns true iff the lock was acquired.
+ */
+bool
+ConditionalLockSharedObject(Oid classid, Oid objid, uint16 objsubid,
+							LOCKMODE lockmode)
+{
+	LOCKTAG		tag;
+
+	SET_LOCKTAG_OBJECT(tag,
+					   InvalidOid,
+					   classid,
+					   objid,
+					   objsubid);
+
+	return (LockAcquire(&tag, lockmode, false, true) != LOCKACQUIRE_NOT_AVAIL);
+}
+
 /*
  *		UnlockSharedObject
  */
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
@@ -103,6 +103,8 @@ extern void UnlockDatabaseObject(Oid classid, Oid objid, uint16 objsubid,
 /* Lock a shared-across-databases object (other than a relation) */
 extern void LockSharedObject(Oid classid, Oid objid, uint16 objsubid,
 							 LOCKMODE lockmode);
+extern bool ConditionalLockSharedObject(Oid classid, Oid objid, uint16 objsubid,
+							 LOCKMODE lockmode);
 extern void UnlockSharedObject(Oid classid, Oid objid, uint16 objsubid,
 							   LOCKMODE lockmode);
 
diff --git a/src/test/isolation2/expected/concurrent_drop_truncate_tablespace.out b/src/test/isolation2/expected/concurrent_drop_truncate_tablespace.out
@@ -1,5 +1,12 @@
 -- While a tablespace is being dropped, if any table is created
--- in the same tablespace, the data of that table should not be deleted
+-- in the same tablespace, only one query can be successful.
+-- The behavior guarantees that the table will never use a
+-- dropped or invalid tablespace.
+
+-- start_matchsubs
+-- m/ERROR:  could not create directory "pg_tblspc.*: No such file or directory/
+-- s/ERROR:  could not create directory "pg_tblspc.*: No such file or directory/ERROR:  could not create directory "pg_tblspc\/XXXX":  No such file or directory/
+-- end_matchsubs
 
 -- create a tablespace directory
 !\retcode rm -rf /tmp/concurrent_tblspace;
@@ -16,51 +23,112 @@
 CREATE TABLESPACE concurrent_tblspace LOCATION '/tmp/concurrent_tblspace';
 CREATE
 
--- suspend execution after TablespaceCreateLock is released
-SELECT gp_inject_fault('AfterTablespaceCreateLockRelease', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+-- test 1:
+-- when creating a table using a tablespace, after the tuple of tablespace
+-- is locked, the tablespace is not allowed to drop
+2: begin;
+BEGIN
+2: CREATE TABLE t_in_tablespace(a int, b int) TABLESPACE concurrent_tblspace;
+CREATE
+
+-- drop tablespace will fail: can't acuqire the lock
+DROP TABLESPACE concurrent_tblspace;
+ERROR:  could not lock tablespace "concurrent_tblspace"
+2: rollback;
+ROLLBACK
+
+-- test 2:
+-- if  DROP TABLESPACE acquires lock first and rollback, the blocking CREATE
+-- TABLE will be successful.
+
+-- suspend execution after tablespace lock is acquired
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
  gp_inject_fault 
 -----------------
  Success:        
  Success:        
  Success:        
 (3 rows)
-1&:DROP TABLESPACE concurrent_tblspace;  <waiting ...>
+1&: DROP TABLESPACE concurrent_tblspace;  <waiting ...>
 
 -- wait for the fault to be triggered
-SELECT gp_wait_until_triggered_fault('AfterTablespaceCreateLockRelease', 1, dbid) from gp_segment_configuration where content <> -1 and role='p';
+SELECT gp_wait_until_triggered_fault('drop_tablespace_after_acquire_lock', 1, dbid) from gp_segment_configuration where content <> -1 and role='p';
  gp_wait_until_triggered_fault 
 -------------------------------
  Success:                      
  Success:                      
  Success:                      
 (3 rows)
 
--- create a table in the same tablespace which is being dropped via a concurrent session
-CREATE TABLE drop_tablespace_tbl(a int, b int) TABLESPACE concurrent_tblspace DISTRIBUTED BY (a);
+2&: CREATE TABLE t_in_tablespace(a int, b int) TABLESPACE concurrent_tblspace;  <waiting ...>
+-- inject an error to ensure that the above DROP command will rollback
+SELECT gp_inject_fault('after_xlog_tblspc_drop', 'error', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+ gp_inject_fault 
+-----------------
+ Success:        
+ Success:        
+ Success:        
+(3 rows)
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+ gp_inject_fault 
+-----------------
+ Success:        
+ Success:        
+ Success:        
+(3 rows)
+-- fail
+1<:  <... completed>
+ERROR:  fault triggered, fault name:'after_xlog_tblspc_drop' fault type:'error'
+-- success
+2<:  <... completed>
 CREATE
-INSERT INTO drop_tablespace_tbl SELECT i, i FROM generate_series(1,100)i;
-INSERT 100
--- reset the fault, drop tablespace command will not delete the data files on the tablespace
-SELECT gp_inject_fault('AfterTablespaceCreateLockRelease', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+-- drop the above table, so the tablespace is empty.
+2: DROP TABLE t_in_tablespace;
+DROP
+SELECT gp_inject_fault('after_xlog_tblspc_drop', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+ gp_inject_fault 
+-----------------
+ Success:        
+ Success:        
+ Success:        
+(3 rows)
+
+-- test 3:
+-- if DROP TABLESPACE acquires lock first and going to drop, any CREATE TABLE
+-- will fail
+
+-- suspend execution after tablespace lock is acquired
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+ gp_inject_fault 
+-----------------
+ Success:        
+ Success:        
+ Success:        
+(3 rows)
+1&: DROP TABLESPACE concurrent_tblspace;  <waiting ...>
+
+-- wait for the fault to be triggered
+SELECT gp_wait_until_triggered_fault('drop_tablespace_after_acquire_lock', 1, dbid) from gp_segment_configuration where content <> -1 and role='p';
+ gp_wait_until_triggered_fault 
+-------------------------------
+ Success:                      
+ Success:                      
+ Success:                      
+(3 rows)
+
+-- create a table in the same tablespace which is being dropped via a concurrent session
+2&:CREATE TABLE drop_tablespace_tbl(a int, b int) TABLESPACE concurrent_tblspace DISTRIBUTED BY (a);  <waiting ...>
+-- reset the fault, drop tablespace command will continue
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
  gp_inject_fault 
 -----------------
  Success:        
  Success:        
  Success:        
 (3 rows)
+-- success
 1<:  <... completed>
 DROP
--- check data exists
-SELECT count(*) FROM drop_tablespace_tbl;
- count 
--------
- 100   
-(1 row)
--- move to another tablespace and check the data.
-ALTER TABLE drop_tablespace_tbl SET TABLESPACE pg_default;
-ALTER
-SELECT count(*) FROM drop_tablespace_tbl;
- count 
--------
- 100   
-(1 row)
+-- fail
+2<:  <... completed>
+ERROR:  could not create directory "pg_tblspc/33175/GPDB_1_302501601/32799": No such file or directory
diff --git a/src/test/isolation2/sql/concurrent_drop_truncate_tablespace.sql b/src/test/isolation2/sql/concurrent_drop_truncate_tablespace.sql
@@ -1,28 +1,70 @@
 -- While a tablespace is being dropped, if any table is created
--- in the same tablespace, the data of that table should not be deleted
+-- in the same tablespace, only one query can be successful.
+-- The behavior guarantees that the table will never use a
+-- dropped or invalid tablespace.
+
+-- start_matchsubs
+-- m/ERROR:  could not create directory "pg_tblspc.*: No such file or directory/
+-- s/ERROR:  could not create directory "pg_tblspc.*: No such file or directory/ERROR:  could not create directory "pg_tblspc\/XXXX":  No such file or directory/
+-- end_matchsubs
 
 -- create a tablespace directory
 !\retcode rm -rf /tmp/concurrent_tblspace;
 !\retcode mkdir -p /tmp/concurrent_tblspace;
 
 CREATE TABLESPACE concurrent_tblspace LOCATION '/tmp/concurrent_tblspace';
 
--- suspend execution after TablespaceCreateLock is released
-SELECT gp_inject_fault('AfterTablespaceCreateLockRelease', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
-1&:DROP TABLESPACE concurrent_tblspace;
+-- test 1:
+-- when creating a table using a tablespace, after the tuple of tablespace
+-- is locked, the tablespace is not allowed to drop
+2: begin;
+2: CREATE TABLE t_in_tablespace(a int, b int) TABLESPACE concurrent_tblspace;
+
+-- drop tablespace will fail: can't acuqire the lock
+DROP TABLESPACE concurrent_tblspace;
+2: rollback;
+
+-- test 2:
+-- if  DROP TABLESPACE acquires lock first and rollback, the blocking CREATE
+-- TABLE will be successful.
+
+-- suspend execution after tablespace lock is acquired
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+1&: DROP TABLESPACE concurrent_tblspace;
+
+-- wait for the fault to be triggered
+SELECT gp_wait_until_triggered_fault('drop_tablespace_after_acquire_lock', 1, dbid)
+   from gp_segment_configuration where content <> -1 and role='p';
+
+2&: CREATE TABLE t_in_tablespace(a int, b int) TABLESPACE concurrent_tblspace;
+-- inject an error to ensure that the above DROP command will rollback
+SELECT gp_inject_fault('after_xlog_tblspc_drop', 'error', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+-- fail
+1<:
+-- success
+2<:
+-- drop the above table, so the tablespace is empty.
+2: DROP TABLE t_in_tablespace;
+SELECT gp_inject_fault('after_xlog_tblspc_drop', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+
+-- test 3:
+-- if DROP TABLESPACE acquires lock first and going to drop, any CREATE TABLE
+-- will fail
+
+-- suspend execution after tablespace lock is acquired
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'suspend', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+1&: DROP TABLESPACE concurrent_tblspace;
 
 -- wait for the fault to be triggered
-SELECT gp_wait_until_triggered_fault('AfterTablespaceCreateLockRelease', 1, dbid)
+SELECT gp_wait_until_triggered_fault('drop_tablespace_after_acquire_lock', 1, dbid)
    from gp_segment_configuration where content <> -1 and role='p';
 
 -- create a table in the same tablespace which is being dropped via a concurrent session
-CREATE TABLE drop_tablespace_tbl(a int, b int) TABLESPACE concurrent_tblspace DISTRIBUTED BY (a);
-INSERT INTO drop_tablespace_tbl SELECT i, i FROM generate_series(1,100)i;
--- reset the fault, drop tablespace command will not delete the data files on the tablespace
-SELECT gp_inject_fault('AfterTablespaceCreateLockRelease', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+2&:CREATE TABLE drop_tablespace_tbl(a int, b int) TABLESPACE concurrent_tblspace DISTRIBUTED BY (a);
+-- reset the fault, drop tablespace command will continue
+SELECT gp_inject_fault('drop_tablespace_after_acquire_lock', 'reset', dbid) FROM gp_segment_configuration WHERE content <> -1 and role='p';
+-- success
 1<:
--- check data exists
-SELECT count(*) FROM drop_tablespace_tbl;
--- move to another tablespace and check the data.
-ALTER TABLE drop_tablespace_tbl SET TABLESPACE pg_default;
-SELECT count(*) FROM drop_tablespace_tbl;
+-- fail
+2<:
diff --git a/src/test/singlenode_isolation2/expected/concurrent_drop_truncate_tablespace.out b/src/test/singlenode_isolation2/expected/concurrent_drop_truncate_tablespace.out
diff --git a/src/test/singlenode_isolation2/sql/concurrent_drop_truncate_tablespace.sql b/src/test/singlenode_isolation2/sql/concurrent_drop_truncate_tablespace.sql
