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

Поиск
Список
Период
Сортировка
От Hayato Kuroda (Fujitsu)
Тема RE: [Proposal] Add foreign-server health checks infrastructure
Дата
Msg-id TYAPR01MB586622DBD4A0C3BA57136899F5B49@TYAPR01MB5866.jpnprd01.prod.outlook.com
обсуждение исходный текст
Ответ на Re: [Proposal] Add foreign-server health checks infrastructure  (vignesh C <vignesh21@gmail.com>)
Ответы Re: [Proposal] Add foreign-server health checks infrastructure  (Katsuragi Yuta <katsuragiy@oss.nttdata.com>)
Список pgsql-hackers
Dear Vignesh,

Thank you for reviewing! PSA new version.

> 
> Few comments:
> 1) There is no handling of forConnCheck in #else HAVE_POLL, if this is
> intentional we could add some comments for the same:
>  static int
> -pqSocketPoll(int sock, int forRead, int forWrite, time_t end_time)
> +pqSocketPoll(int sock, int forRead,
> +                        int forWrite, int forConnCheck, time_t end_time)
>  {
>         /* We use poll(2) if available, otherwise select(2) */
>  #ifdef HAVE_POLL
> @@ -1092,7 +1133,11 @@ pqSocketPoll(int sock, int forRead, int
> forWrite, time_t end_time)
>         int                     timeout_ms;
> 
>         if (!forRead && !forWrite)
> -               return 0;

Comments were added. This missing is intentional, because with the patch present
we do not implement checking feature for kqueue(). If you want to check the
detailed implementation of that, please see 0004 patch attached on [1].

> 2) Can this condition be added to the parent if condition:
>         if (!forRead && !forWrite)
> -               return 0;
> +       {
> +               /* Connection check can be available on some limted platforms
> */
> +               if (!(forConnCheck && PQconnCheckable()))
> +                       return 0;
> +       }

Changed, and added comments atop the if-statement.

> 3) Can we add a comment for PQconnCheckable:
> +/* Check whether the postgres server is still alive or not */
> +extern int PQconnCheck(PGconn *conn);
> +extern int PQconnCheckable(void);
> +

Added. And I found the tab should be used to divide data type and name, so fixed.

> 4) "Note that if 0 is returned and forConnCheck is requested" doesn't
> sound right, it can be changed to "Note that if 0 is returned when
> forConnCheck is requested"
> + * If neither forRead, forWrite nor forConnCheck are set, immediately return a
> + * timeout condition (without waiting). Return >0 if condition is met, 0 if a
> + * timeout occurred, -1 if an error or interrupt occurred. Note that if 0 is
> + * returned and forConnCheck is requested, it means that the socket has not
> + * matched POLLRDHUP event and the connection is not closed by the socket
> peer.

Fixed.

[1]:
https://www.postgresql.org/message-id/TYAPR01MB58669EAAC02493BFF9F39B06F5D99%40TYAPR01MB5866.jpnprd01.prod.outlook.com

Best Regards,
Hayato Kuroda
FUJITSU LIMITED


Вложения

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

Предыдущее
От: John Naylor
Дата:
Сообщение: Re: [PoC] Improve dead tuple storage for lazy vacuum
Следующее
От: Masahiko Sawada
Дата:
Сообщение: Re: Testing autovacuum wraparound (including failsafe)