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

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Katsuragi Yuta' <katsuragiy(at)oss(dot)nttdata(dot)com>
Cc: 'Ted Yu' <yuzhihong(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, vignesh C <vignesh21(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Önder Kalacı <onderkalaci(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, "Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com" <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: 2023-03-07 04:23:00
Message-ID: TYAPR01MB5866F8EA3C06E1B43E42E4E4F5B79@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Katsuragi-san,

Thank you for reviewing! PSA new version patches.

> I think we can update the status to ready for committer after
> this fix, if there is no objection.

That's a very good news for me! How about other people?

> >> 7. the document of postgres_fdw_verify_connection_states_all
> >> <literal>NULL</literal>
> >> + is returned if the local session does not have connection
> >> caches,
> >> or this
> >> + function is not available on this platform.
> >>
> >> I think there is a case where a connection cache exists but valid
> >> connections do not exist and NULL is returned (disconnection case).
> >> Almost the same document as the postgres_fdw_verify_connection_states
> >> case (comment no.5) seems better.
> >
> > Yes, but completely same statement cannot be used because these is not
> > specified foreign server. How about:
> > NULL is returned if there are no established connections or this
> > function ...
>
> Yes, to align with the postgres_fdw_verify_connection_states()
> case, how about writing the connection is not valid. Like the
> following?
> 'NULL is returned if no valid connections are established or
> this function...'

Prefer yours, fixed.

> This is my comment for v35. Please check.
> 0002:
> 1. the comment of verify_cached_connections (I missed one minor point.)
> + * This function emits warnings if a disconnection is found. This
> returns true
> + * if disconnections cannot be found, otherwise returns false.
>
> I think false is returned only if disconnections are found and
> true is returned in all other cases. So, modifying the description
> like the following seems better.
> 'This returns false if disconnections are found, otherwise
> returns true.'

Fixed.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v36-0001-Add-PQconnCheck-and-PQconnCheckable-to-libpq.patch application/octet-stream 8.7 KB
v36-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch application/octet-stream 12.3 KB
v36-0003-add-test.patch application/octet-stream 5.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-03-07 04:32:22 Re: Use pg_pwritev_with_retry() instead of write() in dir_open_for_write() to avoid partial writes?
Previous Message David Rowley 2023-03-07 04:06:42 Re: Making empty Bitmapsets always be NULL