Re: Quorum commit for multiple synchronous replication.

On Mon, Dec 12, 2016 at 9:54 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Mon, Dec 12, 2016 at 9:52 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Mon, Dec 12, 2016 at 9:31 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>> On Sat, Dec 10, 2016 at 5:17 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>>
>>>> Few comments:
>>>
>>> Thank you for reviewing.
>>>
>>>> + * In 10.0 we support two synchronization methods, priority and
>>>> + * quorum. The number of synchronous standbys that transactions
>>>> + * must wait for replies from and synchronization method are specified
>>>> + * in synchronous_standby_names. This parameter also specifies a list
>>>> + * of standby names, which determines the priority of each standby for
>>>> + * being chosen as a synchronous standby. In priority method, the standbys
>>>> + * whose names appear earlier in the list are given higher priority
>>>> + * and will be considered as synchronous. Other standby servers appearing
>>>> + * later in this list represent potential synchronous standbys. If any of
>>>> + * the current synchronous standbys disconnects for whatever reason,
>>>> + * it will be replaced immediately with the next-highest-priority standby.
>>>> + * In quorum method, the all standbys appearing in the list are
>>>> + * considered as a candidate for quorum commit.
>>>>
>>>> In the above description, is priority method represented by FIRST and
>>>> quorum method by ANY in the synchronous_standby_names syntax? If so,
>>>> it might be better to write about it explicitly.
>>>
>>> Added description.
>>>

+ * specified in synchronous_standby_names. The priority method is
+ * represented by FIRST, and the quorum method is represented by ANY

Full stop is missing after "ANY".

>>>
>>>> 6.
>>>> + int sync_method; /* synchronization method */
>>>> /* member_names contains nmembers consecutive nul-terminated C strings */
>>>> char member_names[FLEXIBLE_ARRAY_MEMBER];
>>>> } SyncRepConfigData;
>>>>
>>>> Can't we use 1 or 2 bytes to store sync_method information?
>>>
>>> I changed it to uint8.
>>>

+ int8 sync_method; /* synchronization method */

> I changed it to uint8.

In mail, you have mentioned uint8, but in the code it is int8. I
think you want to update code to use uint8.

>
>> + standby_list { $$ =
>> create_syncrep_config("1", $1, SYNC_REP_PRIORITY); }
>> + | NUM '(' standby_list ')' { $$ =
>> create_syncrep_config($1, $3, SYNC_REP_QUORUM); }
>> + | ANY NUM '(' standby_list ')' { $$ =
>> create_syncrep_config($2, $4, SYNC_REP_QUORUM); }
>> + | FIRST NUM '(' standby_list ')' { $$ =
>> create_syncrep_config($2, $4, SYNC_REP_PRIORITY); }
>>
>> Isn't this "partial" backward-compatibility (i.e., "NUM (list)" works
>> differently from curent version while "list" works in the same way as
>> current one) very confusing?
>>
>> I prefer to either of
>>
>> 1. break the backward-compatibility, i.e., treat the first syntax of
>> "standby_list" as quorum commit
>> 2. keep the backward-compatibility, i.e., treat the second syntax of
>> "NUM (standby_list)" as sync rep with the priority
>

+1.

> There were some comments when I proposed the quorum commit. If we do
> #1 it breaks the backward-compatibility with 9.5 or before as well. I
> don't think it's a good idea. On the other hand, if we do #2 then the
> behaviour of s_s_name is 'NUM (standby_list)' == 'FIRST NUM
> (standby_list)''. But it would not what most of user will want and
> would confuse the users of future version who will want to use the
> quorum commit. Since many hackers thought that the sensible default
> behaviour is 'NUM (standby_list)' == 'ANY NUM (standby_list)' the
> current patch chose to changes the behaviour of s_s_names and document
> that changes thoroughly.
>

Your arguments are sensible, but I think we should address the point
of confusion raised by Fujii-san. As a developer, I feel breaking
backward compatibility (go with Option-1 mentioned above) here is a
good move as it can avoid confusions in future. However, I know many
a time users are so accustomed to the current usage that they feel
irritated with the change in behavior even it is for their betterment,
so it is advisable to do so only if it is necessary or we have
feedback from a couple of users. So in this case, if we don't want to
go with Option-1, then I think we should go with Option-2. If we go
with Option-2, then we can anyway comeback to change the behavior
which is more sensible for future after feedback from users.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com