From: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
---|---|
To: | "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Dave Cramer <pg(at)fastcrypt(dot)com>, Jing Wang <jingwangian(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Libpq support to connect to standby server as priority |
Date: | 2019-04-10 23:13:35 |
Message-ID: | CAJrrPGdUoxFo-QdtdjPF=_x9nYScZnS8OTu_1SOdfNfBsvXQOw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Restructure-the-code-to-remove-duplicate-code.patch | application/octet-stream | 6.3 KB |
0002-New-TargetSessionAttrsType-enum.patch | application/octet-stream | 3.0 KB |
0003-Make-transaction_read_only-as-GUC_REPORT-varaible.patch | application/octet-stream | 9.6 KB |
0004-New-prefer-read-target_session_attrs-type.patch | application/octet-stream | 15.4 KB |
0005-New-read-only-target_session_attrs-type.patch | application/octet-stream | 7.7 KB |
0006-Primary-prefer-standby-and-standby-options.patch | application/octet-stream | 18.9 KB |
0007-New-function-to-rejecting-the-checked-write-connecti.patch | application/octet-stream | 6.2 KB |
0008-Server-recovery-mode-handling.patch | application/octet-stream | 21.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2019-04-10 23:14:12 | Re: serializable transaction: exclude constraint violation (backed by GIST index) instead of ssi conflict |
Previous Message | Peter Geoghegan | 2019-04-10 23:12:43 | Re: Reducing the runtime of the core regression tests |