Re: speed up a logical replica setup

On Thu, Jan 11, 2024, at 9:18 AM, Hayato Kuroda (Fujitsu) wrote:
>
> I have been concerned that the patch has not been tested by cfbot due to the
> application error. Also, some comments were raised. Therefore, I created a patch
> to move forward.
> I also tried to address some comments which is not so claimed by others.
> They were included in 0003 patch.

[I removed the following part in the previous email and couldn't reply to it...]

> * 0001 patch
> It is almost the same as v3-0001, which was posted by Euler.
> An unnecessary change for Mkvcbuild.pm (this file was removed) was ignored.

v5 removes the MSVC support.

> * 0002 patch
> This contains small fixes to keep complier quiet.

I applied it. Although, I used a different approach for format specifier.

> * 0003 patch
> This addresses comments posted to -hackers. For now, this does not contain a doc.
> Will add if everyone agrees these idea.

I didn't review all items but ...

> 1.
> An option --port was added to control the port number for physical standby.
> Users can specify a port number via the option, or an environment variable PGSUBPORT.
> If not specified, a fixed value (50111) would be used.

My first reaction as a new user would be: why do I need to specify a port if my
--subscriber-conninfo already contains a port? Ugh. I'm wondering if we can do
it behind the scenes. Try a range of ports.

> 2.
> A FATAL error would be raised if --subscriber-conninfo specifies non-local server.

Extra protection is always good. However, let's make sure this code path is
really useful. I'll think a bit about it.

> 3.
> Options -o/-O were added to specify options for publications/subscriptions.

Flexibility is cool. However, I think the cost benefit of it is not good. You
have to parse the options to catch preliminary errors. Things like publish only
delete and subscription options that conflicts with the embedded ones are
additional sources of failure.

> 4.
> Made standby to save their output to log file.

It was already done in v5. I did in a different way.

> 5.
> Unnecessary Assert in drop_replication_slot() was removed.

Instead, I fixed the code and keep the assert.

--
Euler Taveira
EDB https://www.enterprisedb.com/