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

On Mon, Nov 29, 2021 at 12:51 AM kuroda(dot)hayato(at)fujitsu(dot)com <
kuroda(dot)hayato(at)fujitsu(dot)com> 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