Re: Quorum commit for multiple synchronous replication.

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Vik Fearing <vik(at)2ndquadrant(dot)fr>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Quorum commit for multiple synchronous replication.
Date: 2016-11-28 11:03:38
Message-ID: CAD21AoCnjxfAcinbh5hK=p5tav3Sxq1UdvDdMqqO4-i2QpjQtA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Nov 26, 2016 at 10:27 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Tue, Nov 15, 2016 at 7:08 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> Attached latest version patch incorporated review comments. After more
>> thought, I agree and changed the value of standby priority in quorum
>> method so that it's not set 1 forcibly. The all standby priorities are
>> 1 If s_s_names = 'ANY(*)'.
>> Please review this patch.
>
> Sorry for my late reply. Here is my final lookup.

Thank you for reviewing!

> <synopsis>
> -<replaceable class="parameter">num_sync</replaceable> ( <replaceable
> class="parameter">standby_name</replaceable> [, ...] )
> +[ANY] <replaceable class="parameter">num_sync</replaceable> (
> <replaceable class="parameter">standby_name</replaceable> [, ...] )
> +FIRST <replaceable class="parameter">num_sync</replaceable> (
> <replaceable class="parameter">standby_name</replaceable> [, ...] )
> <replaceable class="parameter">standby_name</replaceable> [, ...
> This can just be replaced with [ ANY | FIRST ]. There is no need for
> braces as the keyword is not mandatory.
>
> + is the name of a standby server.
> + <literal>FIRST</> and <literal>ANY</> specify the method used by
> + the master to control the standby servres.
> </para>
> s/servres/servers/.
>
> if (priority == 0)
> values[7] = CStringGetTextDatum("async");
> else if (list_member_int(sync_standbys, i))
> - values[7] = CStringGetTextDatum("sync");
> + values[7] = SyncRepConfig->sync_method == SYNC_REP_PRIORITY ?
> + CStringGetTextDatum("sync") : CStringGetTextDatum("quorum");
> else
> values[7] = CStringGetTextDatum("potential");
> This can be simplified a bit as "quorum" is the state value for all
> standbys with a non-zero priority when the method is set to
> SYNC_REP_QUORUM:
> if (priority == 0)
> values[7] = CStringGetTextDatum("async");
> + else if (SyncRepConfig->sync_method == SYNC_REP_QUORUM)
> + values[7] = CStringGetTextDatum("quorum");
> else if (list_member_int(sync_standbys, i))
> values[7] = CStringGetTextDatum("sync");
> else
>
> SyncRepConfig data is made external to syncrep.c with this patch as
> walsender.c needs to look at the sync method in place, no complain
> about that after considering if there could be a more elegant way to
> do things without this change.

Agreed.

> While reviewing the patch, I have found a couple of incorrectly shaped
> sentences, both in the docs and some comments. Attached is a new
> version with this word-smithing. The patch is now switched as ready
> for committer.

Thanks. I found a typo in v7 patch, so attached latest v8 patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachment Content-Type Size
000_quorum_commit_v8.patch application/x-patch 27.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2016-11-28 11:07:05 Re: Mail thread references in commits
Previous Message Masahiko Sawada 2016-11-28 10:37:18 Re: Physical append-only tables