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

From: "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Zhihong Yu' <zyu(at)yugabyte(dot)com>
Cc: Shinya Kato <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: 2021-11-29 08:51:40
Message-ID: TYAPR01MB5866D73C56A4363039175E56F5669@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2021-11-29 09:00:00 Re: Windows: Wrong error message at connection termination
Previous Message houzj.fnst@fujitsu.com 2021-11-29 08:51:35 RE: Data is copied twice when specifying both child and parent table in publication