Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

Поиск
Список
Период
Сортировка
От Ashutosh Sharma
Тема Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints
Дата
Msg-id CAE9k0Pkg20tHq8oiJ+xXa9=af3QZCSYTw99aBaPthA1UMKhnTg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints  (Dilip Kumar <dilipbalaut@gmail.com>)
Ответы Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints  (Dilip Kumar <dilipbalaut@gmail.com>)
Список pgsql-hackers
I see that this patch is reducing the database creation time by almost 3-4 times provided that the template database has some user data in it. However, there are couple of points to be noted:

1) It makes the crash recovery a bit slower than before if the crash has occurred after the execution of a create database statement. Moreover, if the template database size is big, it might even generate a lot of WAL files which the user needs to be aware of.

2) This will put a lot of load on the first checkpoint that will occur after creating the database statement. I will experiment around this to see if this has any side effects.

--

Further, the code changes in the patch looks good. I just have few comments:

+void
+LockRelationId(LockRelId *relid, LOCKMODE lockmode)
+{
+   LOCKTAG     tag;
+   LOCALLOCK  *locallock;
+   LockAcquireResult res;
+
+   SET_LOCKTAG_RELATION(tag, relid->dbId, relid->relId);

Should there be an assertion statement here to ensure that relid->dbid and  relid->relid is valid?

--

    if (info == XLOG_DBASE_CREATE)
    {
        xl_dbase_create_rec *xlrec = (xl_dbase_create_rec *) XLogRecGetData(record);
-       char       *src_path;
-       char       *dst_path;
-       struct stat st;
-
-       src_path = GetDatabasePath(xlrec->src_db_id, xlrec->src_tablespace_id);
-       dst_path = GetDatabasePath(xlrec->db_id, xlrec->tablespace_id);
+       char       *dbpath;

-       /*
-        * Our theory for replaying a CREATE is to forcibly drop the target
-        * subdirectory if present, then re-copy the source data. This may be
-        * more work than needed, but it is simple to implement.
-        */
-       if (stat(dst_path, &st) == 0 && S_ISDIR(st.st_mode))
-       {
-           if (!rmtree(dst_path, true))
-               /* If this failed, copydir() below is going to error. */
-               ereport(WARNING,
-                       (errmsg("some useless files may be left behind in old database directory \"%s\"",
-                               dst_path)));
-       }

I think this is a significant change and probably needs some kind of explanation/comments as-in why we are just creating a dir and copying the version file when replaying create database operation. Earlier, this meant replaying the complete create database operation, that doesn't seem to be the case now.

--

Have you intentionally skipped pg_internal.init file from being copied to the target database?

--
With Regards,
Ashutosh Sharma.


On Thu, Dec 2, 2021 at 7:20 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Wed, Dec 1, 2021 at 6:04 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

> Thanks a lot for testing this. From the error, it seems like some of
> the old buffer w.r.t. the previous tablespace is not dropped after the
> movedb.  Actually, we are calling DropDatabaseBuffers() after copying
> to a new tablespace and dropping all the buffers of this database
> w.r.t the old tablespace.  But seems something is missing, I will
> reproduce this and try to fix it by tomorrow.  I will also fix the
> other review comments raised by you in the previous mail.

Okay, I got the issue, basically we are dropping the database buffers
but not unregistering the existing sync request for database buffers
w.r.t old tablespace. Attached patch fixes that.  I also had to extend
ForgetDatabaseSyncRequests so that we can delete the sync request of
the database for the particular tablespace so added another patch for
the same (0006).

I will test the performance scenario next week, which is suggested by John.


--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Bharath Rupireddy
Дата:
Сообщение: Re: Shouldn't postgres_fdw report warning when it gives up getting result from foreign server?
Следующее
От: Bharath Rupireddy
Дата:
Сообщение: Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?