Re: Review of Refactoring code for sync node detection

On Tue, Nov 18, 2014 at 3:03 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Mon, Nov 17, 2014 at 2:20 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Sun, Nov 16, 2014 at 9:07 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> I'll send an updated patch tomorrow.
>>
>> Here are updated versions. I divided the patch into two for clarity,
>> the first one is the actual refactoring patch, renaming
>> SyncRepGetSynchronousNode to SyncRepGetSynchronousStandby (+alpha,
>> like updating synchronous to sync in the comments as you mentioned)
>> such as the namings have no conflicts.
>>
>> The second one updates the syncrep code, including WAL sender and WAL
>> receiver, and its surroundings to use the term "node" instead of
>> "standby". This brings in the code the idea that a node using
>> replication APIs is not necessarily a standby, making it more generic.
>> This can be applied on top of the refactoring patch. If any other
>> folks (Heikki or Fujii-san?) have comments about this idea feel free.
>
> I've not read the patches yet. But while I was reading the code around
> sync node detection, I thought that it might be good idea to treat the
> node with priority as special. That is, if we find the active node with
> the priority 1, we can immediately go out of the sync-node-detection-loop.
>
> In many cases we can expect that the sync standby has the priority 1.
> If yes, treating the priority 1 as special is good way to do, I think. Thought?

That would really save some cycles. Now we still need to fetch the
priority numbers of all nodes for pg_stat_get_wal_senders, so in this
case scanning all the entries in the WAL sender array is necessary.
This optimization is orthogonal to the refactoring patch btw, no?
-- 
Michael