On Tue, Mar 22, 2022 at 10:28 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
>
> I think this make sense. I haven't changed the original patch as you
> told you were improving on some comments, so in order to avoid
> conflict I have created this add on patch.
>
In my previous patch mistakenly I used src_dboid instead of
dest_dboid. Fixed in this version. For destination db I have used
lock mode as AccessSharedLock. Logically if we see access wise we
don't want anyone else to be accessing that db but that is anyway
protected because it is not visible to anyone else. So I think
AccessSharedLock should be correct here because we are just taking
this lock because we are accessing pages in shared buffers from this
database's relations.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 9636688..49fe104 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -460,12 +460,6 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid, Oid src_tsid, Oid dst_ts
Assert(rnodelist != NIL);
/*
- * Database id is common for all the relation so set it before entering to
- * the loop.
- */
- relid.dbId = src_dboid;
-
- /*
* Iterate over each relfilenode and copy the relation data block by block
* from source database to the destination database.
*/
@@ -488,7 +482,15 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid, Oid src_tsid, Oid dst_ts
dstrnode.dbNode = dst_dboid;
dstrnode.relNode = srcrnode.relNode;
- /* Acquire the lock on relation before start copying. */
+ /*
+ * Acquire relation lock on the source and the destination relation id
+ * before start copying.
+ */
+ relid.dbId = src_dboid;
+ relid.relId = relinfo->reloid;
+ LockRelationId(&relid, AccessShareLock);
+
+ relid.dbId = dst_dboid;
relid.relId = relinfo->reloid;
LockRelationId(&relid, AccessShareLock);
@@ -1218,6 +1220,17 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
InvokeObjectPostCreateHook(DatabaseRelationId, dboid, 0);
/*
+ * Acquire a lock on the target database, although this is a new database
+ * and no one else should be able to see it. But if we are using wal log
+ * strategy then we are going to access the relation pages using shared
+ * buffers. Therefore, it would be better to take the database lock. And,
+ * later we would acquire the relation lock as and when we would access the
+ * individual relations' shared buffers.
+ */
+ if (dbstrategy == CREATEDB_WAL_LOG)
+ LockSharedObject(DatabaseRelationId, dboid, 0, AccessShareLock);
+
+ /*
* 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
* is not a 100% solution, because of the possibility of failure during
@@ -1342,6 +1355,10 @@ createdb_failure_callback(int code, Datum arg)
{
DropDatabaseBuffers(fparms->dest_dboid);
ForgetDatabaseSyncRequests(fparms->dest_dboid);
+
+ /* Release lock on the target database. */
+ UnlockSharedObject(DatabaseRelationId, fparms->dest_dboid, 0,
+ AccessShareLock);
}
/*
