Re: Support for N synchronous standby servers - take 2

At Wed, 20 Apr 2016 11:51:09 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in <CAD21AoC5rrWSk-V79xjVfYr2UqQYrrCKsXkSxZrN9p5YAaeKJA(at)mail(dot)gmail(dot)com>
> On Mon, Apr 18, 2016 at 2:15 PM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > At Fri, 15 Apr 2016 17:36:57 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in <CAD21AoCOL6BCC+FWNCZH_XPgtWc_otnvShMx6_uAcU7Bwb16Rw(at)mail(dot)gmail(dot)com>
> >> How about if check_hook just parses parameter in
> >> CurrentMemoryContext(i.g., T_AllocSetContext), and then the
> >> assign_hook copies syncrep_parse_result to TopMemoryContext.
> >> Because syncrep_parse_result is a global variable, these hooks can see it.
> >
> > Hmm. Somewhat uneasy but should work. The attached patch does it.
..
> Thank you for updating the patch.
>
> Here are some comments.
>
> + Assert(syncrep_parse_result == NULL);
> +
>
> Why do we need Assert at this point?
> It's possible that syncrep_parse_result is not NULL after setting
> s_s_names by ALTER SYSTEM.

Thank you for pointing it out. It is just a trace of an
assumption no longer useful.

> + /*
> + * this memory block will be freed as a part of the
> memory contxt for
> + * config file processing.
> + */
>
> s/contxt/context/

Thanks. I removed whole the comment and the corresponding code
since it's meaningless.

assign_s_s_names causes SEGV when it is called without calling
check_s_s_names. I think that's not the case for this varialbe
because it is unresettable amid a session. It is very uneasy for
me but I don't see a proper means to reset
syncrep_parse_result. MemoryContext deletion hook would work but
it seems to be an overkill for this single use.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center