public inbox for [email protected]
help / color / mirror / Atom feedFrom: Shinya Kato <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
Date: Mon, 06 Dec 2021 20:06:49 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALNJ-vSDLaDnbJpDuKTKwugX+dUnoLD+eEXHdR-aBRXG1Dh3hA@mail.gmail.com>
References: <TYAPR01MB58662809E678253B90E82CE5F5889@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB58662CD4FD98AA475B3D10F9F59B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<[email protected]>
<TYAPR01MB58661992162BA7708191EC79F5619@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CALNJ-vQWNK6AzpL=5iXLfpr4oRMYdJ72tDYhxLwd=iX5H5XUdQ@mail.gmail.com>
<TYAPR01MB5866D73C56A4363039175E56F5669@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<CALNJ-vSDLaDnbJpDuKTKwugX+dUnoLD+eEXHdR-aBRXG1Dh3hA@mail.gmail.com>
On 2021-11-29 21:36, Zhihong Yu wrote:
> On Mon, Nov 29, 2021 at 12:51 AM [email protected]
> <[email protected]> wrote:
>
>> Dear Zhihong,
>>
>> Thank you for giving comments! I'll post new patches later.
>>
>>> +#define HOLD_CHECKING_REMOTE_SERVERS_INTERRUPTS()
>> (CheckingRemoteServersHoldoffCount++)
>>>
>>> The macro contains only one operation. Can the macro be removed
>> (with `CheckingRemoteServersHoldoffCount++` inlined) ?
>>
>> Hmm, these HOLD/RESUME macros are followed HOLD_INTERRUPUST() and
>> HOLD_CANCEL_INTERRUPTS():
>>
>> ```
>> #define HOLD_INTERRUPTS() (InterruptHoldoffCount++)
>>
>> #define RESUME_INTERRUPTS() \
>> do { \
>> Assert(InterruptHoldoffCount > 0); \
>> InterruptHoldoffCount--; \
>> } while(0)
>>
>> #define HOLD_CANCEL_INTERRUPTS() (QueryCancelHoldoffCount++)
>>
>> #define RESUME_CANCEL_INTERRUPTS() \
>> do { \
>> Assert(QueryCancelHoldoffCount > 0); \
>> QueryCancelHoldoffCount--; \
>> } while(0)
>>
>> #define START_CRIT_SECTION() (CritSectionCount++)
>>
>> #define END_CRIT_SECTION() \
>> do { \
>> Assert(CritSectionCount > 0); \
>> CritSectionCount--; \
>> } while(0)
>> ```
>>
>> So I want to keep the current style. Could you tell me if you have
>> any other reasons?
>>
>>> + 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)
>>
>> +1. Will fix.
>>
>> Best Regards,
>> Hayato Kuroda
>> FUJITSU LIMITED
>
> Hi,
>
> It is Okay to keep the macros.
>
> Thanks
Hi, Kuroda-san. Sorry for late reply.
Even for local-only transaction, I thought it useless to execute
CallCheckingRemoteServersCallbacks() and enable_timeout_after(). Can I
make it so that it determines outside whether it contains SQL to the
remote or not?
The following points are minor.
1. In foreign.c, some of the lines are too long and should be broken.
2. In pqcomm.c, the newline have been removed, what is the intention of
this?
3. In postgres.c,
3-1. how about inserting a comment between lines 2713 and 2714, similar
to line 2707?
3-2. the line breaks in line 3242 and line 3243 should be aligned.
3-3. you should change
/*
* Skip checking foreign servers while reading messages.
*/
to
/*
* Skip checking foreign servers while reading messages.
*/
4. In connection.c, There is a typo in line 1684, so "fucntion" should
be changed to "function".
--
Regards,
--
Shinya Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
view thread (39+ 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]
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
In-Reply-To: <[email protected]>
* 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