Re: WIP/PoC for parallel backup

Поиск
Список
Период
Сортировка
От Asif Rehman
Тема Re: WIP/PoC for parallel backup
Дата
Msg-id CADM=Jeg4_-p0KnxmFLauKsOUqgCwP3iy06vdSXSHPa_bzHgCsw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: WIP/PoC for parallel backup  (Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>)
Ответы Re: WIP/PoC for parallel backup
Список pgsql-hackers


On Thu, Oct 17, 2019 at 1:33 AM Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> wrote:
I quickly tried to have a look at your 0001-refactor patch.
Here are some comments:

1. The patch fails to compile.

Sorry if I am missing something, but am not able to understand why in new
function collectTablespaces() you have added an extra parameter NULL while
calling sendTablespace(), it fails the compilation :

+ ti->size = infotbssize ? sendTablespace(fullpath, true, NULL) : -1;


gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -g -O0 -Wall -Werror -I../../../../src/include    -c -o xlog.o xlog.c -MMD -MP -MF .deps/xlog.Po
xlog.c:12253:59: error: too many arguments to function call, expected 2, have 3
                ti->size = infotbssize ? sendTablespace(fullpath, true, NULL) : -1;
                                         ~~~~~~~~~~~~~~                 ^~~~

2. I think the patch needs to run via pg_indent. It does not follow 80 column
width.
e.g.

+void
+collectTablespaces(List **tablespaces, StringInfo tblspcmapfile, bool infotbssize, bool needtblspcmapfile)
+{

3.
The comments in re-factored code appear to be redundant. example:
Following comment:
 /* Setup and activate network throttling, if client requested it */
appears thrice in the code, before calling setup_throttle(), in the prologue of
the function setup_throttle(), and above the if() in that function.
Similarly - the comment:
/* Collect information about all tablespaces */
in collectTablespaces().

4.
In function include_wal_files() why is the parameter TimeLineID i.e. endtli
needed. I don't see it being used in the function at all. I think you can safely
get rid of it.

+include_wal_files(XLogRecPtr endptr, TimeLineID endtli)


Thanks Jeevan. Some changes that should be part of 2nd patch were left in the 1st. I have fixed that and the above mentioned issues as well.
Attached are the updated patches.

Thanks,

--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca

Вложения

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

Предыдущее
От: Thomas Munro
Дата:
Сообщение: Re: SegFault on 9.6.14
Следующее
От: Masahiko Sawada
Дата:
Сообщение: Re: [HACKERS] Block level parallel vacuum