Re: Libpq support to connect to standby server as priority

On Fri, Mar 29, 2019 at 7:06 PM Tsunakawa, Takayuki <
tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:

> Hi Hari-san,
>
> I've reviewed all the files. The patch would be OK when the following
> have been fixed, except for the complexity of fe-connect.c (which probably
> cannot be improved.)
>
> Unfortunately, I'll be absent next week. The earliest date I can do the
> test will be April 8 or 9. I hope someone could take care of this patch...
>

Thanks for the review. Apologies that I could not able finish it on time
because of
other work.

>
> (1) 0001
> With this read-only option type, application can connect to
> to a read-only server in the list of hosts, in case
> ...
> before issuing the SHOW transaction_readonly to find out whether
>
>
> "to" appears twice in a row.
> transaction_readonly -> transaction_read_only
>
>
> (2) 0001
> + succesful connection or failure.
>
> succesful -> successful
>
>
> (3) 0008
> to conenct to a standby server with a faster check instead of
>
> conenct -> connect
>
>
> (4) 0008
> Logically, recovery exit should be notified after the following statement:
>
> XLogCtl->SharedRecoveryInProgress = false;
>
>
> (5) 0008
> + /* Update in_recovery status. */
> + if (LocalRecoveryInProgress)
> + SetConfigOption("in_recovery",
> + "on",
> + PGC_INTERNAL,
> PGC_S_OVERRIDE);
> +
>
> This SetConfigOption() is called for every RecoveryInProgress() call on
> the standby. It may cause undesirable overhead. How about just calling
> SetConfigOption() once in InitPostgres() to set the initial value for
> in_recovery? InitPostgres() and its subsidiary functions call
> SetConfigOption() likewise.
>
>
> (6) 0008
> async.c is for LISTEN/UNLISTEN/NOTIFY. How about adding the new functions
> in postgres.c like RecoveryConflictInterrupt()?
>
>
> (7) 0008
> + if (pid != 0)
> + {
> + (void) SendProcSignal(pid, reason,
> procvxid.backendId);
> + }
>
> The braces are not necessary because the block only contains a single
> statement.
>

I fixed all the comments that you have raised above and attached the updated
patches.

Regards,
Haribabu Kommi
Fujitsu Australia