public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: [email protected] <[email protected]>
To: 'Önder Kalacı' <[email protected]>
To: Fujii Masao <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: Mon, 17 Oct 2022 03:02:05 +0000
Message-ID: <TYCPR01MB83739440BAD5DC46CFA696CAED299@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB5866CE34430424A588F60FC2F55D9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <[email protected]>
<TYAPR01MB58664E79C4728A6FFEA46F94F53B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB5866FF28C802916242F1B492F53B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB5866C6278B6386672B35F624F53B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB5866FC683843ED8BD09505FEF53D9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB5866982FC21ABE5DF3C1E3A2F5059@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB58661CD72B69848FBE0CD466F54F9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB586690F8E0FC4AAC0C1A32A6F55B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CACawEhW_R=6mKsB24QW3WpCZTQgtxAPH7J0q8yedKkCQY2xT0w@mail.gmail.com>
<TYAPR01MB5866F419C4261177578AC1CCF55B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CACawEhXAy4gjJ4G3-59Xv_yb8Xbtn1VMYP8SOj-0C9dG2zbAcQ@mail.gmail.com>
<TYAPR01MB5866ACB4D3B845B4B2ED121BF55A9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB5866CE34430424A588F60FC2F55D9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
On Wednesday, October 5, 2022 6:27 PM [email protected] <[email protected]> wrote:
> Thanks for giving many comments! Based on off and on discussions, I modified
> my patch.
Here are my other quick review comments on v16.
(1) v16-0001 : definition of a new structure
CheckingRemoteServersCallbackItem can be added to typedefs.list.
(2) v16-0001 : UnRegisterCheckingRemoteServersCallback's header comment
+void
+UnRegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback callback,
+ void *arg)
+{
Looks require a header comment for this function,
because in this file, all other functions have each one.
(3) v16-0002 : better place to declare new variables
New variables 'old' and 'server' in GetConnection() can be moved to
a scope of 'if' statement where those are used.
@@ -138,6 +149,8 @@ GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state)
ConnCacheEntry *entry;
ConnCacheKey key;
MemoryContext ccxt = CurrentMemoryContext;
+ MemoryContext old;
+ ForeignServer *server = GetForeignServer(user->serverid);
(4) v16-0002 : typo in pgfdw_connection_check_internal()
+ /* return false is if the socket seems to be closed. */
It should be "return false if the socket seems to be closed" ?
(5) v16-0002 : pgfdw_connection_check's message
When I tested the new feature, I got a below message.
"ERROR: Foreign Server my_external_server might be down."
I think we can wrap the server name with double quotes
so that the message is more aligned with existing error message
like connect_pg_server.
(6) v16-003 : typo
+ Authors needs to register the callback function via following API.
Should be "Authors need to register the ...".
(7) v16-0004 : unnecessary file
I think we can remove a new file which looks like a log file.
.../postgres_fdw/expected/postgres_fdw_1.out
Best Regards,
Takamichi Osumi
view thread (47+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
In-Reply-To: <TYCPR01MB83739440BAD5DC46CFA696CAED299@TYCPR01MB8373.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox