Re: Support for N synchronous standby servers

On Fri, Aug 22, 2014 at 7:14 PM, Rajeev rastogi <rajeev(dot)rastogi(at)huawei(dot)com>
wrote:

> I have just started looking into this patch.
> Please find below my first level of observation from the patch:
>

Thanks! Updated patch attached.

> 1. Allocation of memory for sync_nodes in function
> SyncRepGetSynchronousNodes should be equivalent to allowed_sync_nodes
> instead of max_wal_senders. As anyway we are not going to store sync stdbys
> more than allowed_sync_nodes.
> sync_nodes = (int *) palloc(allowed_sync_nodes *
> sizeof(int));
>

Fixed.

2. Logic of deciding the highest priority one seems to be in-correct.
> Assume, s_s_num = 3, s_s_names = 3,4,2,1
> standby nodes are in order as: 1,2,3,4,5,6,7
>
> As per the logic in patch, node 4 with priority 2 will not be
> added in the list whereas 1,2,3 will be added.
>
> The problem is because priority updated for next tracking is not
> the highest priority as of that iteration, it is just priority of last node
> added to the list. So it may happen that a node with higher priority
> is still there in list but we are comparing with some other smaller
> priority.
>

Fixed. Nice catch!

> 3. Can we optimize the function SyncRepGetSynchronousNodes in such a way
> that it gets the number of standby nodes from s_s_names itself. With this
> it will be usful to stop scanning the moment we get first s_s_num potential
> standbys.
>

By doing so, we would need to scan the WAL sender array more than once (or
once if we can find N sync nodes with a name matching the first entry, smth
unlikely to happen). We would need as well to recalculate for a given item
in the list _names what is its priority and compare it with the existing
entries in the WAL sender list. So this is not worth the shot.
Also, using the priority instead of s_s_names is more solid as s_s_names is
now used only in SyncRepGetStandbyPriority to calculate the priority for a
given WAL sender, and is a function only called by a WAL sender itself when
it initializes.
Regards,
--
Michael