From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Cc: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Maciek Sakrejda <m(dot)sakrejda(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |
Date: | 2022-03-14 16:34:04 |
Message-ID: | CA+TgmobpcwVtQ2y+N0yKK9YCDb+9akDNyXWyOhb1AT1srZQ1oA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 14, 2022 at 12:04 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Mar 14, 2022 at 7:51 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > Other than that, I have fixed some mistakes in comments and supported
> > tab completion for the new options.
>
> I was looking at 0001 and 0002 again and realized that I swapped the
> names load_relmap_file() and read_relmap_file() from what I should
> have done. Here's a revised version. With this, read_relmap_file() and
> write_relmap_file() become functions that just read and write the file
> without touching any global variables, and load_relmap_file() is the
> function that reads data from the file and puts it into a global
> variable, which seems more sensible than the way I had it before.
Regarding 0003 and 0005, I'm not a fan of 'bool isunlogged'. I think
'bool permanent' would be better (note BM_PERMANENT). This would
involve reversing true and false.
Regarding 0004, I can't really see a reason for this function to take
a LockRelId as a parameter rather than two separate OIDs. I also can't
entirely see why it should be called LockRelationId. Maybe
LockRelationInDatabaseById(Oid dbid, Oid relid, LOCKMODE lockmode)?
Note that neither caller actually has a LockRelId available; both have
to construct one.
Regarding 0005:
+ CREATEDB_WAL_LOG = 0,
+ CREATEDB_FILE_COPY = 1
I still think you don't need = 0 and = 1 here.
I'll probably go through and do a pass over the comments once you post
the next version of this. There seems to be work needed in a bunch of
places, but it probably makes more sense for me to go through and
adjust the things that seem to need it rather than listing a bunch of
changes for you to make.
--
Robert Haas
EDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2022-03-14 16:35:47 | Re: refactoring basebackup.c (zstd workers) |
Previous Message | Imseih (AWS), Sami | 2022-03-14 16:20:51 | Re: Add index scan progress to pg_stat_progress_vacuum |