Re: Support for N synchronous standby servers - take 2

Thank you for the revised patch.

At Tue, 22 Mar 2016 16:02:39 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
<CAHGQGwGnvuX8wR-FYH+TrNi_TWunZzU=nJFMdXkO6O8M4GbNvQ@mail.gmail.com>
> On Thu, Mar 10, 2016 at 7:21 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> Sorry for the delay... Here is the revised version of the patch.
> Please review and test this version!
> BTW, I've not revised the documentation and regression test yet.
> I will do that during the review and test of the patch.

This version looks to focus on n-priority method. Stuffs for the
other methods like n-quorum has been removed. It is okay for me.

So using WalSnd->sync_standby_priority is reasonable. 

SyncRePGetSyncStandbys seems to work as expected, that is,
collecting n standbys in the order of priority, even if multiple
standbys are at the same prioirity, but in (pseudo) random order
among the standbys with the same priority, not LSN order. This is
the difference from the true quoraum method.

About announcement of take over,

>   if (announce_next_takeover && am_sync)
>   {
>     announce_next_takeover = false;
>     ereport(LOG,
>         (errmsg("standby \"%s\" is now the synchronous standby with priority %u",
>             application_name, MyWalSnd->sync_standby_priority)));

This can announces for the seemingly same standby successively if
standbys with the same application_name are comming-in and
going-out. But this is the same as the current behavior.

Otherwise, as far as I can see, SyncRepReleaseWaiters seems to
work correctly.


SyncRepinitConfig parses s_s_names then prioritize all walsenders
based on the result. This is run at the start of a walsender and
at reloading of config. Ended walsenders are excluded on
collectiong sync-standbys. All of these seems to work
properly. (as before).

The parser became far simpler by getting rid of the stuffs for
the future expansion. It accepts only '<n>[name, ...]' and the
old s_s_names format.

StringInfo for double-quoted names seems to me to be overkill,
since it allocates 1024 byte block for every such name. A static
buffer seems enough for the usage as I said.

The parser is called for not only for SIGHUP, but also for
starting of every walsender. The latter is not necessary but it
is the matter of trade-off between simplisity and
effectiveness. The same can be said for
check_synchronous_standby_names().

regards,


-- 
Kyotaro Horiguchi
NTT Open Source Software Center