Re: [Proposal] Add foreign-server health checks infrastructure

Поиск
Список
Период
Сортировка
От Zhihong Yu
Тема Re: [Proposal] Add foreign-server health checks infrastructure
Дата
Msg-id CALNJ-vQWNK6AzpL=5iXLfpr4oRMYdJ72tDYhxLwd=iX5H5XUdQ@mail.gmail.com
обсуждение исходный текст
Ответ на RE: [Proposal] Add foreign-server health checks infrastructure  ("kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>)
Ответы RE: [Proposal] Add foreign-server health checks infrastructure  ("kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>)
Список pgsql-hackers


On Tue, Nov 23, 2021 at 8:57 PM kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com> wrote:
Dear Kato-san,

Thank you for reviewing!

> Thank you for sending the patches!
> I confirmed that they can be compiled and tested successfully on CentOS
> 8.

Thanks!

> +     {
> +             {"remote_servers_connection_check_interval", PGC_USERSET,
> CONN_AUTH_SETTINGS,
> +                     gettext_noop("Sets the time interval between checks
> for
> disconnection of remote servers."),
> +                     NULL,
> +                     GUC_UNIT_MS
> +             },
> +             &remote_servers_connection_check_interval,
> +             0, 0, INT_MAX,
> +     },
>
> If you don't use check_hook, assign_hook and show_hook, you should
> explicitly write "NULL, NULL, NULL", as show below.

Yeah I forgot the line. Fixed.

> +                     ereport(ERROR,
> +
>       errcode(ERRCODE_CONNECTION_FAILURE),
> +                                     errmsg("Postgres foreign server %s
> might be down.",
> +
>       server->servername));
>
> According to [1], error messages should start with a lowercase letter
> and not use a period.
> Also, along with the rest of the code, it is a good idea to enclose the
> server name in double quotes.

I confirmed the postgres error-reporting policy and fixed to follow that.
How do you think?

Attached are the latest version.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Hi,

+#define HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS()  (CheckingRemoteServersHoldoffCount++)

The macro contains only one operation. Can the macro be removed (with `CheckingRemoteServersHoldoffCount++` inlined) ?

+   if (CheckingRemoteServersTimeoutPending && CheckingRemoteServersHoldoffCount != 0)
+   {
+       /*
+        * Skip checking foreign servers while reading messages.
+        */
+       InterruptPending = true;
+   }
+   else if (CheckingRemoteServersTimeoutPending)

Would the code be more readable if the above two if blocks be moved inside one enclosing if block (factoring the common condition)?

+   if (CheckingRemoteServersTimeoutPending)

Cheers

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

Предыдущее
От: Sasasu
Дата:
Сообщение: [PATCH] buffile: ensure start offset is aligned with BLCKSZ
Следующее
От: SATYANARAYANA NARLAPURAM
Дата:
Сообщение: Switching XLog source from archive to streaming when primary available