On Fri, Mar 29, 2019 at 7:06 PM Tsunakawa, Takayuki < tsunakawa.ta...@jp.fujitsu.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
0001-Restructure-the-code-to-remove-duplicate-code.patch
Description: Binary data
0002-New-TargetSessionAttrsType-enum.patch
Description: Binary data
0003-Make-transaction_read_only-as-GUC_REPORT-varaible.patch
Description: Binary data
0004-New-prefer-read-target_session_attrs-type.patch
Description: Binary data
0005-New-read-only-target_session_attrs-type.patch
Description: Binary data
0006-Primary-prefer-standby-and-standby-options.patch
Description: Binary data
0007-New-function-to-rejecting-the-checked-write-connecti.patch
Description: Binary data
0008-Server-recovery-mode-handling.patch
Description: Binary data
